[SYCL] Remove generic_type_lists.hpp
#15714
Conversation
We don't need to instantiate all the `vec`/`marray` types to implement `generic_type_traits.hpp`.
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| inline __SYCL_ALWAYS_INLINE std::enable_if_t< | ||
| sycl::detail::is_contained< | ||
| T, sycl::ext::oneapi::experimental::cuda::detail::ldg_types>::value, | ||
| detail::check_type_in_v<detail::element_type_t<T>, char, signed char, short, |
There was a problem hiding this comment.
I think this part isn't NFC in a sense that gtl::scalar_floating_list on the left included bfloat16 but the code in this file hasn't been adjusted to actually handle it when it was added there. As such, the change here is actually good.
There was a problem hiding this comment.
Should we leave a TODO reminding it on code?
There was a problem hiding this comment.
IMO, that's up to the @intel/llvm-reviewers-cuda to decide if any changes are necessary here.
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
maarquitos14
left a comment
maarquitos14
left a comment
There was a problem hiding this comment.
LGTM, just one nit.
| inline __SYCL_ALWAYS_INLINE std::enable_if_t< | ||
| sycl::detail::is_contained< | ||
| T, sycl::ext::oneapi::experimental::cuda::detail::ldg_types>::value, | ||
| detail::check_type_in_v<detail::element_type_t<T>, char, signed char, short, |
There was a problem hiding this comment.
Should we leave a TODO reminding it on code?
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| template <typename T> | ||
| inline constexpr bool is_sgenfloat_v = | ||
| is_contained_v<T, gtl::scalar_floating_list>; | ||
| check_type_in_v<T, float, double, half, ext::oneapi::bfloat16>; |
There was a problem hiding this comment.
given that you replace is_contained_v with check_type_in_v its implementation should probably be changed to the is_base_of way of checking membership for best performance
There was a problem hiding this comment.
Yeah, I'll look into that separately.
|
@npmiller , @intel/llvm-reviewers-cuda , ping, you're the last approval needed. |
4 participants
We don't need to instantiate all the
vec/marraytypes to implementgeneric_type_traits.hpp.