Skip to content

[SYCL] Implement proper queries for aspect::ext_oneapi_srgb #9243

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 28, 2023
Merged

[SYCL] Implement proper queries for aspect::ext_oneapi_srgb #9243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a21c059
[SYCL] Implement proper queries for aspect::ext_oneapi_srgb
aelovikov-intel Apr 27, 2023
6a1387e
Fix build
aelovikov-intel Apr 27, 2023
1cbbcb4
Fix the test failure
aelovikov-intel Apr 28, 2023
82a4b78
Address code review
aelovikov-intel Apr 28, 2023
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
6 changes: 5 additions & 1 deletion sycl/plugins/cuda/pi_cuda.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2105,6 +2105,11 @@ pi_result cuda_piDeviceGetInfo(pi_device device, pi_device_info param_name,
return getInfo<pi_bool>(param_value_size, param_value, param_value_size_ret,
false);
}
case PI_DEVICE_INFO_IMAGE_SRGB: {
// The sRGB images are not supported on CUDA.
return getInfo<pi_bool>(param_value_size, param_value, param_value_size_ret,
false);
}

// TODO: Investigate if this information is available on CUDA.
case PI_DEVICE_INFO_PCI_ADDRESS:
Expand All @@ -2114,7 +2119,6 @@ pi_result cuda_piDeviceGetInfo(pi_device device, pi_device_info param_name,
case PI_DEVICE_INFO_GPU_SUBSLICES_PER_SLICE:
case PI_DEVICE_INFO_GPU_EU_COUNT_PER_SUBSLICE:
case PI_DEVICE_INFO_GPU_HW_THREADS_PER_EU:
case PI_DEVICE_INFO_IMAGE_SRGB:
return PI_ERROR_INVALID_VALUE;

default:
Expand Down
4 changes: 3 additions & 1 deletion sycl/plugins/esimd_emulator/pi_esimd_emulator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,9 @@ pi_result piDeviceGetInfo(pi_device Device, pi_device_info ParamName,
case PI_EXT_INTEL_DEVICE_INFO_MEM_CHANNEL_SUPPORT:
// The mem-channel buffer property is not supported on the ESIMD emulator.
return ReturnValue(pi_bool{false});
case PI_DEVICE_INFO_IMAGE_SRGB:
// The sRGB images are not supported on the ESIMD emulator.
return ReturnValue(pi_bool{false});

CASE_PI_UNSUPPORTED(PI_DEVICE_INFO_SUB_GROUP_INDEPENDENT_FORWARD_PROGRESS)
CASE_PI_UNSUPPORTED(PI_DEVICE_INFO_IL_VERSION)
Expand All @@ -829,7 +832,6 @@ pi_result piDeviceGetInfo(pi_device Device, pi_device_info ParamName,
CASE_PI_UNSUPPORTED(PI_DEVICE_INFO_GPU_SUBSLICES_PER_SLICE)
CASE_PI_UNSUPPORTED(PI_DEVICE_INFO_GPU_EU_COUNT_PER_SUBSLICE)
CASE_PI_UNSUPPORTED(PI_DEVICE_INFO_MAX_MEM_BANDWIDTH)
CASE_PI_UNSUPPORTED(PI_DEVICE_INFO_IMAGE_SRGB)
CASE_PI_UNSUPPORTED(PI_DEVICE_INFO_ATOMIC_64)
CASE_PI_UNSUPPORTED(PI_EXT_DEVICE_INFO_ATOMIC_MEMORY_ORDER_CAPABILITIES)
CASE_PI_UNSUPPORTED(PI_EXT_DEVICE_INFO_ATOMIC_MEMORY_SCOPE_CAPABILITIES)
Expand Down
6 changes: 5 additions & 1 deletion sycl/plugins/hip/pi_hip.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1960,6 +1960,11 @@ pi_result hip_piDeviceGetInfo(pi_device device, pi_device_info param_name,
return getInfo<pi_bool>(param_value_size, param_value, param_value_size_ret,
false);
}
case PI_DEVICE_INFO_IMAGE_SRGB: {
// The sRGB images are not supported on HIP device.
return getInfo<pi_bool>(param_value_size, param_value, param_value_size_ret,
false);
}

// TODO: Investigate if this information is available on HIP.
case PI_DEVICE_INFO_PCI_ADDRESS:
Expand All @@ -1971,7 +1976,6 @@ pi_result hip_piDeviceGetInfo(pi_device device, pi_device_info param_name,
case PI_DEVICE_INFO_GPU_HW_THREADS_PER_EU:
case PI_DEVICE_INFO_MAX_MEM_BANDWIDTH:
case PI_EXT_ONEAPI_DEVICE_INFO_BFLOAT16_MATH_FUNCTIONS:
case PI_DEVICE_INFO_IMAGE_SRGB:
setErrorMessage("HIP backend does not support this query",
PI_ERROR_INVALID_ARG_VALUE);
return PI_ERROR_PLUGIN_SPECIFIC_ERROR;
Expand Down
3 changes: 3 additions & 0 deletions sycl/plugins/unified_runtime/pi2ur.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,9 @@ inline pi_result piDeviceGetInfo(pi_device Device, pi_device_info ParamName,
case PI_EXT_INTEL_DEVICE_INFO_MEM_CHANNEL_SUPPORT:
InfoType = (ur_device_info_t)UR_EXT_DEVICE_INFO_MEM_CHANNEL_SUPPORT;
break;
case PI_DEVICE_INFO_IMAGE_SRGB:
InfoType = (ur_device_info_t)UR_DEVICE_INFO_IMAGE_SRGB;
break;
default:
return PI_ERROR_UNKNOWN;
};
Expand Down
2 changes: 2 additions & 0 deletions sycl/plugins/unified_runtime/ur/adapters/level_zero/ur_level_zero.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,8 @@ ur_result_t urDeviceGetInfo(
}
case UR_EXT_DEVICE_INFO_MEM_CHANNEL_SUPPORT:
return ReturnValue(pi_bool{false});
case UR_DEVICE_INFO_IMAGE_SRGB:
return ReturnValue(pi_bool{false});

// TODO: Implement.
default:
Expand Down
14 changes: 14 additions & 0 deletions sycl/test-e2e/Basic/image/srgba-aspect-check.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out
// RUN: %CPU_RUN_PLACEHOLDER %t.out
// RUN: %GPU_RUN_PLACEHOLDER %t.out
// RUN: %ACC_RUN_PLACEHOLDER %t.out

#include <sycl/sycl.hpp>

int main() {
sycl::queue q;
// Ensure that plugins that don't implement the support can still handle the
// aspect query.
std::ignore = q.get_device().has(sycl::aspect::ext_oneapi_srgb);
return 0;
}
5 changes: 2 additions & 3 deletions sycl/test-e2e/Basic/image/srgba-read.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@
// RUN: %CPU_RUN_PLACEHOLDER %t.out %CPU_CHECK_PLACEHOLDER
// RUN: %GPU_RUN_PLACEHOLDER %t.out %GPU_CHECK_PLACEHOLDER

// XFAIL: level_zero
// UNSUPPORTED: cuda
// UNSUPPORTED: hip
// Only opencl plugin has implemented the supoort.
// REQUIRES: opencl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the aspect check works, should we maybe just do the check and an early exit instead? In the future we would that to a REQUIRES: aspect_image && aspect_ext_oneapi_srgb or something to that effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of this test is FileCheck which we can't nicely implement without dynamic LIT features support.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. Good point. Should we have a TODO just to remind us?


#include <iostream>
#include <sycl/sycl.hpp>
Expand Down