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

[SYCL][Docs] Add legacy SYCL 1.2.1 image aspect #9217

Merged
merged 5 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions sycl/doc/PreprocessorMacros.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ This file describes macros that have effect on SYCL compiler and run-time.
Disables all deprecation warnings in SYCL runtime headers, including SYCL
1.2.1 deprecations.

- **SYCL_DISABLE_IMAGE_ASPECT_WARNING**

Disables warning diagnostic issued when calling `device::has(aspect::image)`
and `platform::has(aspect::image)`.

- **SYCL_FALLBACK_ASSERT**

Defining as non-zero enables the fallback assert feature even on devices
Expand Down
112 changes: 112 additions & 0 deletions sycl/doc/extensions/supported/sycl_ext_intel_legacy_image.asciidoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
= sycl_ext_intel_legacy_image

:source-highlighter: coderay
:coderay-linenums-mode: table

// This section needs to be after the document title.
:doctype: book
:toc2:
:toc: left
:encoding: utf-8
:lang: en
:dpcpp: pass:[DPC++]

// Set the default source code type in this document to C++,
// for syntax highlighting purposes. This is needed because
// docbook uses c++ and html5 uses cpp.
:language: {basebackend@docbook:c++:cpp}


== Notice

[%hardbreaks]
Copyright (C) 2023-2023 Intel Corporation. All rights reserved.

Khronos(R) is a registered trademark and SYCL(TM) and SPIR(TM) are trademarks
of The Khronos Group Inc. OpenCL(TM) is a trademark of Apple Inc. used by
permission by Khronos.


== Contact

To report problems with this extension, please open a new issue at:

https://github.com/intel/llvm/issues


== Dependencies

This extension is written against the SYCL 2020 revision 6 specification. All
references below to the "core SYCL specification" or to section numbers in the
SYCL specification refer to that revision.


== Status

This extension is implemented and fully supported by {dpcpp}.


== Overview

SYCL 2020 removed the SYCL 1.2.1 `image` class as well as the associated image
accessors and `sampler` class. However, the device info query for
`sycl::info::device::image_support` stayed in SYCL 2020 as deprecated. As the
specification states that this query returns the same value as
`device::has(aspect::image)`, the user can no longer query support for the SYCL
1.2.1 images in implementations that support these.

This extension adds the new aspect `sycl::aspect::ext_intel_legacy_image`
intended for querying if a device supports SYCL 1.2.1 images.


== Specification

=== Feature test macro

This extension provides a feature-test macro as described in the core SYCL
specification. An implementation supporting this extension must predefine the
macro `SYCL_EXT_INTEL_LEGACY_IMAGE` to one of the values defined in the table
below. Applications can test for the existence of this macro to determine if
the implementation supports this feature, or applications can test the macro's
value to determine which of the extension's features the implementation
supports.

[%header,cols="1,5"]
|===
|Value
|Description

|1
|Initial version of this extension.
|===


=== The `ext_intel_legacy_image` aspect

This extension adds the following new value to the SYCL `aspect` enum:

```
namespace sycl {

enum class aspect {
...
ext_intel_legacy_image
};

} // namespace sycl
```

The new aspect has the following behaviour when queried via `device::has()`:

[%header,cols="1,5"]
|===
|Aspect
|Description

|`aspect::ext_intel_legacy_image`
|Indicates that the device supports SYCL 1.2.1 image accessor, as defined in
section 4.7.6.11 to 4.7.6.13 of the SYCL 1.2.1 Specification Rev. 7, and
samplers, as defined in section 4.7.8 and 4.7.9 of the SYCL 1.2.1 Specification
Rev. 7.
|===

15 changes: 6 additions & 9 deletions sycl/include/sycl/accessor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -665,13 +665,6 @@ class image_accessor
static_assert(Dimensions > 0 && Dimensions <= 3,
"Dimensions can be 1/2/3 for image accessor.");

template <typename Param>
void checkDeviceFeatureSupported(const device &Device) {
if (!Device.get_info<Param>())
throw feature_not_supported("Images are not supported by this device.",
PI_ERROR_INVALID_OPERATION);
}

#ifdef __SYCL_DEVICE_ONLY__

sycl::vec<int, Dimensions> getRangeInternal() const {
Expand Down Expand Up @@ -773,8 +766,12 @@ class image_accessor
MImageCount(ImageRef.size()),
MImgChannelOrder(ImageRef.getChannelOrder()),
MImgChannelType(ImageRef.getChannelType()) {
checkDeviceFeatureSupported<info::device::image_support>(
getDeviceFromHandler(CommandGroupHandlerRef));

device Device = getDeviceFromHandler(CommandGroupHandlerRef);
if (!Device.has(aspect::ext_intel_legacy_image))
throw feature_not_supported(
"SYCL 1.2.1 images are not supported by this device.",
PI_ERROR_INVALID_OPERATION);
}
#endif

Expand Down
14 changes: 14 additions & 0 deletions sycl/include/sycl/detail/defines_elementary.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@
#endif
#endif // __SYCL2020_DEPRECATED

#ifndef __SYCL_WARN_IMAGE_ASPECT
#if !defined(SYCL_DISABLE_IMAGE_ASPECT_WARNING) && __has_attribute(diagnose_if)
#define __SYCL_WARN_IMAGE_ASPECT(aspect_param) \
__attribute__((diagnose_if( \
aspect_param == aspect::image, \
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens here if the value of aspect_param is not known at compile time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sadly it would not produce the warning. For example a call like

sycl::aspect A = sycl::aspect::image;
Dev.has(A);

would not produce the warning. However, the expectation is that using the aspect directly is the most common pattern and as such should catch the majority of cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's OK. I was worried that the compiler might diagnose an error if the expression aspect_param == aspect::image cannot be evaluated at compile time.

"SYCL 2020 images are not supported on any devices. Consider using " \
"‘aspect::ext_intel_legacy_image’ instead. Disable this warning with " \
"by defining SYCL_DISABLE_IMAGE_ASPECT_WARNING.", \
"warning")))
#else
#define __SYCL_WARN_IMAGE_ASPECT(aspect)
#endif
#endif // __SYCL_WARN_IMAGE_ASPECT

#ifndef __SYCL_HAS_CPP_ATTRIBUTE
#if defined(__cplusplus) && defined(__has_cpp_attribute)
#define __SYCL_HAS_CPP_ATTRIBUTE(x) __has_cpp_attribute(x)
Expand Down
2 changes: 1 addition & 1 deletion sycl/include/sycl/device.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ class __SYCL_EXPORT device : public detail::OwnerLessBase<device> {
/// Provisional Spec.
///
/// \return true if the SYCL device has the given feature.
bool has(aspect Aspect) const;
bool has(aspect Aspect) const __SYCL_WARN_IMAGE_ASPECT(Aspect);

private:
std::shared_ptr<detail::device_impl> impl;
Expand Down
10 changes: 10 additions & 0 deletions sycl/include/sycl/device_aspect_macros.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@
#define __SYCL_ALL_DEVICES_HAVE_40__ 0
#endif

#ifndef __SYCL_ALL_DEVICES_HAVE_41__
// __SYCL_ASPECT(ext_intel_legacy_image, 41)
#define __SYCL_ALL_DEVICES_HAVE_41__ 0
#endif

#ifndef __SYCL_ANY_DEVICE_HAS_0__
// __SYCL_ASPECT(host, 0)
#define __SYCL_ANY_DEVICE_HAS_0__ 0
Expand Down Expand Up @@ -417,3 +422,8 @@
// __SYCL_ASPECT(emulated, 40)
#define __SYCL_ANY_DEVICE_HAS_40__ 0
#endif

#ifndef __SYCL_ANY_DEVICE_HAS_41__
// __SYCL_ASPECT(ext_intel_legacy_image, 41)
#define __SYCL_ANY_DEVICE_HAS_41__ 0
#endif
6 changes: 6 additions & 0 deletions sycl/include/sycl/device_aspect_traits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ struct all_devices_have<aspect::ext_intel_memory_bus_width>
template <>
struct all_devices_have<aspect::emulated>
: std::bool_constant<__SYCL_ALL_DEVICES_HAVE_40__> {};
template <>
struct all_devices_have<aspect::ext_intel_legacy_image>
: std::bool_constant<__SYCL_ALL_DEVICES_HAVE_41__> {};

#ifdef __SYCL_ANY_DEVICE_HAS_ANY_ASPECT__
// Special case where any_device_has is trivially true.
Expand Down Expand Up @@ -264,6 +267,9 @@ struct any_device_has<aspect::ext_intel_memory_bus_width>
template <>
struct any_device_has<aspect::emulated>
: std::bool_constant<__SYCL_ANY_DEVICE_HAS_40__> {};
template <>
struct any_device_has<aspect::ext_intel_legacy_image>
: std::bool_constant<__SYCL_ANY_DEVICE_HAS_41__> {};
#endif // __SYCL_ANY_DEVICE_HAS_ANY_ASPECT__

template <aspect Aspect>
Expand Down
1 change: 1 addition & 0 deletions sycl/include/sycl/info/aspects.def
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@ __SYCL_ASPECT(ext_intel_device_id, 37)
__SYCL_ASPECT(ext_intel_memory_clock_rate, 38)
__SYCL_ASPECT(ext_intel_memory_bus_width, 39)
__SYCL_ASPECT(emulated, 40)
__SYCL_ASPECT(ext_intel_legacy_image, 41)
2 changes: 1 addition & 1 deletion sycl/include/sycl/info/device_traits_deprecated.def
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Marked deprecated in SYCL 2020 spec
__SYCL_PARAM_TRAITS_DEPRECATED(image_support, "deprecated in SYCL 2020, use device::has(aspect::image) instead")
__SYCL_PARAM_TRAITS_DEPRECATED(image_support, "deprecated in SYCL 2020, use device::has(aspect::ext_intel_legacy_image) to query for SYCL 1.2.1 image support")
__SYCL_PARAM_TRAITS_DEPRECATED(max_constant_buffer_size,"deprecated in SYCL 2020")
__SYCL_PARAM_TRAITS_DEPRECATED(max_constant_args,"deprecated in SYCL 2020")
__SYCL_PARAM_TRAITS_DEPRECATED(host_unified_memory,"deprecated in SYCL 2020, use device::has() with "
Expand Down
2 changes: 1 addition & 1 deletion sycl/include/sycl/platform.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class __SYCL_EXPORT platform : public detail::OwnerLessBase<platform> {
///
/// \return true if all of the SYCL devices on this platform have the
/// given feature.
bool has(aspect Aspect) const;
bool has(aspect Aspect) const __SYCL_WARN_IMAGE_ASPECT(Aspect);

/// Return this platform's default context
///
Expand Down
8 changes: 8 additions & 0 deletions sycl/source/detail/device_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,14 @@ bool device_impl::has(aspect Aspect) const {
&async_barrier_supported, nullptr) == PI_SUCCESS;
return call_successful && async_barrier_supported;
}
case aspect::ext_intel_legacy_image: {
pi_bool legacy_image_support = PI_FALSE;
bool call_successful =
getPlugin().call_nocheck<detail::PiApiKind::piDeviceGetInfo>(
MDevice, PI_DEVICE_INFO_IMAGE_SUPPORT, sizeof(pi_bool),
&legacy_image_support, nullptr) == PI_SUCCESS;
return call_successful && legacy_image_support;
}
default:
throw runtime_error("This device aspect has not been implemented yet.",
PI_ERROR_INVALID_DEVICE);
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Basic/image/srgba-read.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ int main() {
if (D.has(aspect::ext_oneapi_srgb))
std::cout << "aspect::ext_oneapi_srgb detected" << std::endl;

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// RGBA -- (normal, non-linearized)
std::cout << "rgba -------" << std::endl;
test_rd(image_channel_order::rgba, image_channel_type::unorm_int8);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ int main(int Argc, const char *Argv[]) {
sycl::queue Q(props);

auto dev = Q.get_device();
if (dev.has(aspect::image)) {
if (dev.has(aspect::ext_intel_legacy_image)) {
if (TestType == "image") {
std::cerr << "RunTest_ImageTest" << std::endl;
RunTest_ImageTest(Q);
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/basic-rw-float.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/basic-rw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/normalized-clamp-linear-float.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/normalized-clamp-nearest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/normalized-clampedge-nearest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/normalized-mirror-linear-float.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/normalized-mirror-nearest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/normalized-none-linear-float.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/normalized-none-nearest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/normalized-repeat-linear-float.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
2 changes: 1 addition & 1 deletion sycl/test-e2e/Sampler/normalized-repeat-nearest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ int main() {
queue Q;
device D = Q.get_device();

if (D.has(aspect::image)) {
if (D.has(aspect::ext_intel_legacy_image)) {
// the _int8 channels are one byte per channel, or four bytes per pixel (for
// RGBA) the _int16/fp16 channels are two bytes per channel, or eight bytes
// per pixel (for RGBA) the _int32/fp32 channels are four bytes per
Expand Down
Loading