From cf524596eafb3ccee9b21ea916446b67d8b61222 Mon Sep 17 00:00:00 2001 From: Nicolas Miller Date: Wed, 20 Nov 2024 11:47:15 +0000 Subject: [PATCH 1/2] [HIP] Fix error code for unsupported program info Unsupported program info should return unsupported enumeration instead of unsupported feature. This aligns with the CUDA adapter and makes the CTS understand that these properties aren't failing but just unsupported. --- source/adapters/hip/program.cpp | 5 ++++- test/conformance/program/program_adapter_hip.match | 3 --- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source/adapters/hip/program.cpp b/source/adapters/hip/program.cpp index 4c4f2b2766..d7b7be10fe 100644 --- a/source/adapters/hip/program.cpp +++ b/source/adapters/hip/program.cpp @@ -408,7 +408,10 @@ urProgramGetInfo(ur_program_handle_t hProgram, ur_program_info_t propName, case UR_PROGRAM_INFO_BINARIES: return ReturnValue(&hProgram->Binary, 1); case UR_PROGRAM_INFO_KERNEL_NAMES: - return getKernelNames(hProgram); + /* TODO: Add implementation for getKernelNames */ + UR_ASSERT(getKernelNames(hProgram), UR_RESULT_ERROR_UNSUPPORTED_FEATURE); + return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION; + case UR_PROGRAM_INFO_NUM_KERNELS: case UR_PROGRAM_INFO_IL: return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION; default: diff --git a/test/conformance/program/program_adapter_hip.match b/test/conformance/program/program_adapter_hip.match index 69fe6ac1bb..2bf556541d 100644 --- a/test/conformance/program/program_adapter_hip.match +++ b/test/conformance/program/program_adapter_hip.match @@ -1,9 +1,6 @@ urProgramBuildTest.BuildFailure/* # HIP hasn't implemented urProgramCreateWithNativeHandleTest {{OPT}}urProgramCreateWithNativeHandleTest.Success/* -# HIP doesn't expose kernel numbers or names -urProgramGetInfoTest.Success/*__UR_PROGRAM_INFO_NUM_KERNELS -urProgramGetInfoTest.Success/*__UR_PROGRAM_INFO_KERNEL_NAMES # HIP hasn't implemented urProgramLink {{OPT}}urProgramLinkTest.Success/* From 8644396213b31979c7744fbf26deb105afed5ed0 Mon Sep 17 00:00:00 2001 From: Nicolas Miller Date: Wed, 20 Nov 2024 14:10:17 +0000 Subject: [PATCH 2/2] [CUDA][HIP] Cleanup KERNEL_NAMES property No need to keep an empty function, it can always be added if we need it in the future. --- source/adapters/cuda/program.cpp | 16 +++------------- source/adapters/hip/program.cpp | 14 +++----------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/source/adapters/cuda/program.cpp b/source/adapters/cuda/program.cpp index 4b963a737a..dfd5e9e6b8 100644 --- a/source/adapters/cuda/program.cpp +++ b/source/adapters/cuda/program.cpp @@ -176,17 +176,6 @@ ur_result_t ur_program_handle_t_::getGlobalVariablePointer( return UR_RESULT_SUCCESS; } -/// Finds kernel names by searching for entry points in the PTX source, as the -/// CUDA driver API doesn't expose an operation for this. -/// Note: This is currently only being used by the SYCL program class for the -/// has_kernel method, so an alternative would be to move the has_kernel -/// query to UR and use cuModuleGetFunction to check for a kernel. -/// Note: Another alternative is to add kernel names as metadata, like with -/// reqd_work_group_size. -ur_result_t getKernelNames(ur_program_handle_t) { - return UR_RESULT_ERROR_UNSUPPORTED_FEATURE; -} - /// Loads images from a list of PTX or CUBIN binaries. /// Note: No calls to CUDA driver API in this function, only store binaries /// for later. @@ -421,8 +410,9 @@ urProgramGetInfo(ur_program_handle_t hProgram, ur_program_info_t propName, case UR_PROGRAM_INFO_BINARIES: return ReturnValue(&hProgram->Binary, 1); case UR_PROGRAM_INFO_KERNEL_NAMES: - /* TODO: Add implementation for getKernelNames */ - UR_ASSERT(getKernelNames(hProgram), UR_RESULT_ERROR_UNSUPPORTED_FEATURE); + // CUDA has no way to query a list of kernels from a binary. + // In SYCL this is only used in kernel bundle when building from source + // which isn't currently supported for CUDA. return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION; case UR_PROGRAM_INFO_NUM_KERNELS: case UR_PROGRAM_INFO_IL: diff --git a/source/adapters/hip/program.cpp b/source/adapters/hip/program.cpp index d7b7be10fe..eae3fda366 100644 --- a/source/adapters/hip/program.cpp +++ b/source/adapters/hip/program.cpp @@ -259,15 +259,6 @@ ur_result_t ur_program_handle_t_::getGlobalVariablePointer( return UR_RESULT_SUCCESS; } -/// Finds kernel names by searching for entry points in the PTX source, as the -/// HIP driver API doesn't expose an operation for this. -/// Note: This is currently only being used by the SYCL program class for the -/// has_kernel method, so an alternative would be to move the has_kernel -/// query to UR and use hipModuleGetFunction to check for a kernel. -ur_result_t getKernelNames(ur_program_handle_t) { - return UR_RESULT_ERROR_UNSUPPORTED_FEATURE; -} - /// A program must be specific to a device so this entry point is UNSUPPORTED UR_APIEXPORT ur_result_t UR_APICALL urProgramCreateWithIL(ur_context_handle_t, const void *, size_t, @@ -408,8 +399,9 @@ urProgramGetInfo(ur_program_handle_t hProgram, ur_program_info_t propName, case UR_PROGRAM_INFO_BINARIES: return ReturnValue(&hProgram->Binary, 1); case UR_PROGRAM_INFO_KERNEL_NAMES: - /* TODO: Add implementation for getKernelNames */ - UR_ASSERT(getKernelNames(hProgram), UR_RESULT_ERROR_UNSUPPORTED_FEATURE); + // HIP has no way to query a list of kernels from a binary. + // In SYCL this is only used in kernel bundle when building from source + // which isn't currently supported for HIP. return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION; case UR_PROGRAM_INFO_NUM_KERNELS: case UR_PROGRAM_INFO_IL: