Skip to content

[NFC][SYCL] Use get_elem_type_t instead of TryToGetElementType<T>::type #12738

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 2 commits into from
Feb 20, 2024
Merged

[NFC][SYCL] Use get_elem_type_t instead of TryToGetElementType<T>::type #12738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06081d4
[NFC][SYCL] Use get_elem_type_t instead of TryToGetElementType<T>::type
aelovikov-intel Feb 16, 2024
975b732
Merge remote-tracking branch 'origin/sycl' into HEAD
aelovikov-intel Feb 20, 2024
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
4 changes: 1 addition & 3 deletions sycl/include/sycl/accessor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1121,9 +1121,7 @@ class __image_array_slice__ {

constexpr static int AdjustedDims = (Dimensions == 2) ? 4 : Dimensions + 1;

template <typename CoordT,
typename CoordElemType =
typename detail::TryToGetElementType<CoordT>::type>
template <typename CoordT, typename CoordElemType = get_elem_type_t<CoordT>>
sycl::vec<CoordElemType, AdjustedDims>
getAdjustedCoords(const CoordT &Coords) const {
CoordElemType LastCoord = 0;
Expand Down
24 changes: 12 additions & 12 deletions sycl/include/sycl/detail/generic_type_traits.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,18 +301,6 @@ template <typename T>
using make_unsinged_integer_t =
make_type_t<T, gtl::scalar_unsigned_integer_list>;

// TryToGetElementType<T>::type is T::element_type or T::value_type if those
// exist, otherwise T.
template <typename T> class TryToGetElementType {
static T check(...);
template <typename A> static typename A::element_type check(const A &);
template <typename A> static typename A::value_type check(const A &);

public:
using type = decltype(check(T()));
static constexpr bool value = !std::is_same_v<T, type>;
};

// select_apply_cl_scalar_t selects from T8/T16/T32/T64 basing on
// sizeof(IN). expected to handle scalar types.
template <typename T, typename T8, typename T16, typename T32, typename T64>
Expand Down Expand Up @@ -525,6 +513,18 @@ template <typename T, typename Enable = void> struct RelConverter {
static R apply(value_t value) { return value; }
};

// TryToGetElementType<T>::type is T::element_type or T::value_type if those
// exist, otherwise T.
template <typename T> class TryToGetElementType {
static T check(...);
template <typename A> static typename A::element_type check(const A &);
template <typename A> static typename A::value_type check(const A &);

public:
using type = decltype(check(T()));
static constexpr bool value = !std::is_same_v<T, type>;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we keep TryToGetElementType?

Copy link
Contributor Author

@aelovikov-intel aelovikov-intel Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It has a single remaining use just below at line 650. Note that it's ::value, not ::type so I can't do the same replacement as in other places of this PR. I think it's possible to replace that as well somehow, but I've decided to wait until we can drop legacy builtins implementation and remove it after that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I personally think value in this is unintuitive, so I would prefer we replace it, if nothing else then by just replacing the use with !std::is_same_v<T, get_elem_type_t<T>>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not immediately obvious if that would be an NFC or not, so I won't be doing it here.


template <typename T>
struct RelConverter<T,
typename std::enable_if_t<TryToGetElementType<T>::value>> {
Expand Down
6 changes: 2 additions & 4 deletions sycl/include/sycl/detail/image_accessor_util.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,8 +773,6 @@ void imageWriteHostImpl(const CoordT &Coords, const WriteDataT &Color,
break;
case image_channel_type::fp16:
writePixel(
// convertWriteDataToHalf<typename
// TryToGetElementType<WriteDataT>::type>(
convertWriteData<half>(Color, ImgChannelType),
reinterpret_cast<half *>(Ptr), ImgChannelOrder, ImgChannelType);
break;
Expand Down Expand Up @@ -915,7 +913,7 @@ DataT getColor(const int4 PixelCoord, const addressing_mode SmplAddrMode,
DataT RetData;
if (isOutOfRange(PixelCoord, SmplAddrMode, ImgRange)) {
float4 BorderColor = getBorderColor(ImgChannelOrder);
RetData = BorderColor.convert<typename TryToGetElementType<DataT>::type>();
RetData = BorderColor.convert<get_elem_type_t<DataT>>();
} else {
RetData = ReadPixelData<DataT>(PixelCoord, ImgPitch, ImgChannelType,
ImgChannelOrder, BasePtr, ElementSize);
Expand Down Expand Up @@ -984,7 +982,7 @@ DataT ReadPixelDataLinearFiltMode(const int8 CoordValues, const float4 abc,
// (1 – a) * b * Ci0j1 + a * b * Ci1j1;
// For 1D image: j0 = 0, j1 = 0, k0 = 0, k1 = 0, b = 0.5, c = 0.5.
// RetData = (1 – a) * Ci0 + a * Ci1;
return RetData.convert<typename TryToGetElementType<DataT>::type>();
return RetData.convert<get_elem_type_t<DataT>>();
}

// imageReadSamplerHostImpl method is called by the read API in image accessors
Expand Down
6 changes: 3 additions & 3 deletions sycl/test-e2e/Basic/image/image_accessor_readwrite.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ template <typename WriteDataT, int ImgType, int read_write> class kernel_class;

template <typename ReadDataT,
typename = typename std::enable_if<
(!(std::is_same<ReadDataT, s::cl_float4>::value) &&
!(std::is_same<ReadDataT, s::cl_half4>::value))>::type>
(!(std::is_same_v<ReadDataT, s::cl_float4>) &&
!(std::is_same_v<ReadDataT, s::cl_half4>))>::type>
void check_read_data(ReadDataT ReadData, ReadDataT ExpectedColor) {
using ReadDataType = typename s::detail::TryToGetElementType<ReadDataT>::type;
using ReadDataType = typename ReadDataT::element_type;
bool CorrectData = false;
if ((ReadData.x() == ExpectedColor.x()) &&
(ReadData.y() == ExpectedColor.y()) &&
Expand Down