Skip to content

Commit

Permalink
[Spec Constants] Improved handling of invalid spec. constants
Browse files Browse the repository at this point in the history
Two main changes to how `Kernel/ProgramSetSpecializationConstants`
are handled:
* They may now output either `INVALID_VALUE` or the new
  `INVALID_SPEC_ID` when the provided list is invalid.
* The OpenCL adapter now responds to
  `UR_DEVICE_INFO_KERNEL_SET_SPECIALIZATION_CONSTANTS` with `false`
  rather than erroring out. This fixes some tests that were
  incorrectly not being skipped.

Note that specialization constants are not yet supported on CUDA,
so these tests fail there.
  • Loading branch information
RossBrunton committed Mar 22, 2024
1 parent a504ead commit ccc0846
Show file tree
Hide file tree
Showing 12 changed files with 97 additions and 1 deletion.
11 changes: 11 additions & 0 deletions include/ur_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,7 @@ typedef enum ur_result_t {
UR_RESULT_ERROR_ADAPTER_SPECIFIC = 67, ///< An adapter specific warning/error has been reported and can be
///< retrieved via the urPlatformGetLastError entry point.
UR_RESULT_ERROR_LAYER_NOT_PRESENT = 68, ///< A requested layer was not found by the loader.
UR_RESULT_ERROR_INVALID_SPEC_ID = 69, ///< A specialization constant identifier is not valid.
UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP = 0x1000, ///< Invalid Command-Buffer
UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_SYNC_POINT_EXP = 0x1001, ///< Sync point is not valid for the command-buffer
UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_SYNC_POINT_WAIT_LIST_EXP = 0x1002, ///< Sync point wait list is invalid
Expand Down Expand Up @@ -4534,6 +4535,11 @@ typedef struct ur_specialization_constant_info_t {
/// + `NULL == pSpecConstants`
/// - ::UR_RESULT_ERROR_INVALID_SIZE
/// + `count == 0`
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// + Any size specified in a pSpecConstant entry does not match the specialization constant in the module.
/// + Any pValue specified in a pSpecConstant entry is nullptr.
/// - ::UR_RESULT_ERROR_INVALID_SPEC_ID
/// + Any id specified in a pSpecConstant entry is not a valid specialization constant identifier.
UR_APIEXPORT ur_result_t UR_APICALL
urProgramSetSpecializationConstants(
ur_program_handle_t hProgram, ///< [in] handle of the Program object
Expand Down Expand Up @@ -5118,6 +5124,11 @@ urKernelSetArgMemObj(
/// + `count == 0`
/// - ::UR_RESULT_ERROR_UNSUPPORTED_FEATURE
/// + If ::UR_DEVICE_INFO_KERNEL_SET_SPECIALIZATION_CONSTANTS query is false
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// + Any size specified in a pSpecConstant entry does not match the specialization constant in the module.
/// + Any pValue specified in a pSpecConstant entry is nullptr.
/// - ::UR_RESULT_ERROR_INVALID_SPEC_ID
/// + Any id specified in a pSpecConstant entry is not a valid specialization constant identifier.
UR_APIEXPORT ur_result_t UR_APICALL
urKernelSetSpecializationConstants(
ur_kernel_handle_t hKernel, ///< [in] handle of the kernel object
Expand Down
3 changes: 3 additions & 0 deletions include/ur_print.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1539,6 +1539,9 @@ inline std::ostream &operator<<(std::ostream &os, enum ur_result_t value) {
case UR_RESULT_ERROR_LAYER_NOT_PRESENT:
os << "UR_RESULT_ERROR_LAYER_NOT_PRESENT";
break;
case UR_RESULT_ERROR_INVALID_SPEC_ID:
os << "UR_RESULT_ERROR_INVALID_SPEC_ID";
break;
case UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP:
os << "UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP";
break;
Expand Down
2 changes: 2 additions & 0 deletions scripts/core/common.yml
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,8 @@ etors:
via the urPlatformGetLastError entry point."
- name: ERROR_LAYER_NOT_PRESENT
desc: "A requested layer was not found by the loader."
- name: ERROR_INVALID_SPEC_ID
desc: "A specialization constant identifier is not valid."
- name: ERROR_UNKNOWN
value: "0x7ffffffe"
desc: "Unknown or internal error"
Expand Down
5 changes: 5 additions & 0 deletions scripts/core/kernel.yml
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,11 @@ returns:
- "`count == 0`"
- $X_RESULT_ERROR_UNSUPPORTED_FEATURE:
- "If $X_DEVICE_INFO_KERNEL_SET_SPECIALIZATION_CONSTANTS query is false"
- $X_RESULT_ERROR_INVALID_VALUE:
- "Any size specified in a pSpecConstant entry does not match the specialization constant in the module."
- "Any pValue specified in a pSpecConstant entry is nullptr."
- $X_RESULT_ERROR_INVALID_SPEC_ID:
- "Any id specified in a pSpecConstant entry is not a valid specialization constant identifier."
--- #--------------------------------------------------------------------------
type: function
desc: "Return platform native kernel handle."
Expand Down
5 changes: 5 additions & 0 deletions scripts/core/program.yml
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,11 @@ params:
returns:
- $X_RESULT_ERROR_INVALID_SIZE:
- "`count == 0`"
- $X_RESULT_ERROR_INVALID_VALUE:
- "Any size specified in a pSpecConstant entry does not match the specialization constant in the module."
- "Any pValue specified in a pSpecConstant entry is nullptr."
- $X_RESULT_ERROR_INVALID_SPEC_ID:
- "Any id specified in a pSpecConstant entry is not a valid specialization constant identifier."
--- #--------------------------------------------------------------------------
type: function
desc: "Return program native program handle."
Expand Down
2 changes: 2 additions & 0 deletions source/adapters/opencl/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ ur_result_t mapCLErrorToUR(cl_int Result) {
return UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP;
case CL_INVALID_SYNC_POINT_WAIT_LIST_KHR:
return UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_SYNC_POINT_WAIT_LIST_EXP;
case CL_INVALID_SPEC_ID:
return UR_RESULT_ERROR_INVALID_SPEC_ID;
default:
return UR_RESULT_ERROR_UNKNOWN;
}
Expand Down
4 changes: 3 additions & 1 deletion source/adapters/opencl/device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,6 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
case UR_DEVICE_INFO_COMPILER_AVAILABLE:
case UR_DEVICE_INFO_LINKER_AVAILABLE:
case UR_DEVICE_INFO_PREFERRED_INTEROP_USER_SYNC:
case UR_DEVICE_INFO_KERNEL_SET_SPECIALIZATION_CONSTANTS:
case UR_DEVICE_INFO_SUB_GROUP_INDEPENDENT_FORWARD_PROGRESS: {
/* CL type: cl_bool
* UR type: ur_bool_t */
Expand Down Expand Up @@ -969,6 +968,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
case UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP: {
return ReturnValue(false);
}
case UR_DEVICE_INFO_KERNEL_SET_SPECIALIZATION_CONSTANTS: {
return ReturnValue(false);
}
default: {
return UR_RESULT_ERROR_INVALID_ENUMERATION;
}
Expand Down
10 changes: 10 additions & 0 deletions source/loader/ur_libapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3402,6 +3402,11 @@ ur_result_t UR_APICALL urProgramGetBuildInfo(
/// + `NULL == pSpecConstants`
/// - ::UR_RESULT_ERROR_INVALID_SIZE
/// + `count == 0`
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// + Any size specified in a pSpecConstant entry does not match the specialization constant in the module.
/// + Any pValue specified in a pSpecConstant entry is nullptr.
/// - ::UR_RESULT_ERROR_INVALID_SPEC_ID
/// + Any id specified in a pSpecConstant entry is not a valid specialization constant identifier.
ur_result_t UR_APICALL urProgramSetSpecializationConstants(
ur_program_handle_t hProgram, ///< [in] handle of the Program object
uint32_t count, ///< [in] the number of elements in the pSpecConstants array
Expand Down Expand Up @@ -3987,6 +3992,11 @@ ur_result_t UR_APICALL urKernelSetArgMemObj(
/// + `count == 0`
/// - ::UR_RESULT_ERROR_UNSUPPORTED_FEATURE
/// + If ::UR_DEVICE_INFO_KERNEL_SET_SPECIALIZATION_CONSTANTS query is false
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// + Any size specified in a pSpecConstant entry does not match the specialization constant in the module.
/// + Any pValue specified in a pSpecConstant entry is nullptr.
/// - ::UR_RESULT_ERROR_INVALID_SPEC_ID
/// + Any id specified in a pSpecConstant entry is not a valid specialization constant identifier.
ur_result_t UR_APICALL urKernelSetSpecializationConstants(
ur_kernel_handle_t hKernel, ///< [in] handle of the kernel object
uint32_t count, ///< [in] the number of elements in the pSpecConstants array
Expand Down
10 changes: 10 additions & 0 deletions source/ur_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2897,6 +2897,11 @@ ur_result_t UR_APICALL urProgramGetBuildInfo(
/// + `NULL == pSpecConstants`
/// - ::UR_RESULT_ERROR_INVALID_SIZE
/// + `count == 0`
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// + Any size specified in a pSpecConstant entry does not match the specialization constant in the module.
/// + Any pValue specified in a pSpecConstant entry is nullptr.
/// - ::UR_RESULT_ERROR_INVALID_SPEC_ID
/// + Any id specified in a pSpecConstant entry is not a valid specialization constant identifier.
ur_result_t UR_APICALL urProgramSetSpecializationConstants(
ur_program_handle_t hProgram, ///< [in] handle of the Program object
uint32_t count, ///< [in] the number of elements in the pSpecConstants array
Expand Down Expand Up @@ -3385,6 +3390,11 @@ ur_result_t UR_APICALL urKernelSetArgMemObj(
/// + `count == 0`
/// - ::UR_RESULT_ERROR_UNSUPPORTED_FEATURE
/// + If ::UR_DEVICE_INFO_KERNEL_SET_SPECIALIZATION_CONSTANTS query is false
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// + Any size specified in a pSpecConstant entry does not match the specialization constant in the module.
/// + Any pValue specified in a pSpecConstant entry is nullptr.
/// - ::UR_RESULT_ERROR_INVALID_SPEC_ID
/// + Any id specified in a pSpecConstant entry is not a valid specialization constant identifier.
ur_result_t UR_APICALL urKernelSetSpecializationConstants(
ur_kernel_handle_t hKernel, ///< [in] handle of the kernel object
uint32_t count, ///< [in] the number of elements in the pSpecConstants array
Expand Down
20 changes: 20 additions & 0 deletions test/conformance/kernel/urKernelSetSpecializationConstants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,23 @@ TEST_P(urKernelSetSpecializationConstantsTest, InvalidSizeCount) {
ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_SIZE,
urKernelSetSpecializationConstants(kernel, 0, &info));
}

TEST_P(urKernelSetSpecializationConstantsTest, InvalidValueSize) {
ur_specialization_constant_info_t bad_info = {0, 0x1000, &spec_value};
ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_VALUE,
urKernelSetSpecializationConstants(kernel, 1, &bad_info));
}

TEST_P(urKernelSetSpecializationConstantsTest, InvalidValueId) {
ur_specialization_constant_info_t bad_info = {999, sizeof(spec_value),
&spec_value};
ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_SPEC_ID,
urKernelSetSpecializationConstants(kernel, 1, &bad_info));
}

TEST_P(urKernelSetSpecializationConstantsTest, InvalidValuePtr) {
ur_specialization_constant_info_t bad_info = {0, sizeof(spec_value),
nullptr};
ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_VALUE,
urKernelSetSpecializationConstants(kernel, 1, &bad_info));
}
3 changes: 3 additions & 0 deletions test/conformance/program/program_adapter_cuda.match
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@
{{OPT}}urProgramGetInfoTest.InvalidNullHandleProgram/NVIDIA_CUDA_BACKEND___{{.*}}___UR_PROGRAM_INFO_KERNEL_NAMES
{{OPT}}urProgramLinkTest.Success/NVIDIA_CUDA_BACKEND___{{.*}}_
{{OPT}}urProgramSetSpecializationConstantsTest.Success/NVIDIA_CUDA_BACKEND___{{.*}}_
{{OPT}}urProgramSetSpecializationConstantsTest.InvalidValueSize/NVIDIA_CUDA_BACKEND___{{.*}}_
{{OPT}}urProgramSetSpecializationConstantsTest.InvalidValueId/NVIDIA_CUDA_BACKEND___{{.*}}_
{{OPT}}urProgramSetSpecializationConstantsTest.InvalidValuePtr/NVIDIA_CUDA_BACKEND___{{.*}}_
23 changes: 23 additions & 0 deletions test/conformance/program/urProgramSetSpecializationConstants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,26 @@ TEST_P(urProgramSetSpecializationConstantsTest, InvalidSizeCount) {
ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_SIZE,
urProgramSetSpecializationConstants(program, 0, &info));
}

TEST_P(urProgramSetSpecializationConstantsTest, InvalidValueSize) {
ur_specialization_constant_info_t bad_info = {0, 0x1000, &spec_value};
ASSERT_EQ_RESULT(
UR_RESULT_ERROR_INVALID_VALUE,
urProgramSetSpecializationConstants(program, 1, &bad_info));
}

TEST_P(urProgramSetSpecializationConstantsTest, InvalidValueId) {
ur_specialization_constant_info_t bad_info = {999, sizeof(spec_value),
&spec_value};
ASSERT_EQ_RESULT(
UR_RESULT_ERROR_INVALID_SPEC_ID,
urProgramSetSpecializationConstants(program, 1, &bad_info));
}

TEST_P(urProgramSetSpecializationConstantsTest, InvalidValuePtr) {
ur_specialization_constant_info_t bad_info = {0, sizeof(spec_value),
nullptr};
ASSERT_EQ_RESULT(
UR_RESULT_ERROR_INVALID_VALUE,
urProgramSetSpecializationConstants(program, 1, &bad_info));
}

0 comments on commit ccc0846

Please sign in to comment.