Skip to content

[SYCL] Infer IdT in GroupBroadcast from group type #2115

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

Merged
merged 2 commits into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
16 changes: 15 additions & 1 deletion sycl/include/CL/sycl/detail/spirv.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,29 @@ template <typename Group> bool GroupAny(bool pred) {
}

// Broadcast with scalar local index
// Work-group supports any integral type
// Sub-group currently supports only uint32_t
template <typename Group, typename T, typename IdT>
detail::enable_if_t<std::is_integral<IdT>::value, T>
detail::enable_if_t<is_group<Group>::value && std::is_integral<IdT>::value, T>
GroupBroadcast(T x, IdT local_id) {
using OCLT = detail::ConvertToOpenCLType_t<T>;
using OCLIdT = detail::ConvertToOpenCLType_t<IdT>;
OCLT ocl_x = detail::convertDataToType<T, OCLT>(x);
OCLIdT ocl_id = detail::convertDataToType<IdT, OCLIdT>(local_id);
return __spirv_GroupBroadcast(group_scope<Group>::value, ocl_x, ocl_id);
}
template <typename Group, typename T, typename IdT>
detail::enable_if_t<is_sub_group<Group>::value && std::is_integral<IdT>::value,
Copy link
Contributor

@v-klochkov v-klochkov Jul 14, 2020

Choose a reason for hiding this comment

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

If IdT is uint64_t, then it will silently truncated to uint32_t, right?
Would it be useful here to assert that local_id fits into 32-bits, or throw an exception if local_id does not fit into 32-bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Truncation is the desired behavior here.

A sub-group's local ID is represented by an id<1> (which is a size_t) but the CPU runtime only accepts a SPIR-V OpGroupBroadcast instruction with Subgroup scope if the LocalId parameter is a uint32_t.

This version of GroupBroadcast can only be called if the Group is a sub-group, so truncating into 32 bits should always be safe.

T>
GroupBroadcast(T x, IdT local_id) {
using SGIdT = uint32_t;
SGIdT sg_local_id = static_cast<SGIdT>(local_id);
using OCLT = detail::ConvertToOpenCLType_t<T>;
using OCLIdT = detail::ConvertToOpenCLType_t<SGIdT>;
OCLT ocl_x = detail::convertDataToType<T, OCLT>(x);
OCLIdT ocl_id = detail::convertDataToType<SGIdT, OCLIdT>(sg_local_id);
return __spirv_GroupBroadcast(group_scope<Group>::value, ocl_x, ocl_id);
}

// Broadcast with vector local index
template <typename Group, typename T, int Dimensions>
Expand Down
18 changes: 18 additions & 0 deletions sycl/include/CL/sycl/detail/type_traits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

__SYCL_INLINE_NAMESPACE(cl) {
namespace sycl {
template <int Dimensions> class group;
namespace intel {
struct sub_group;
} // namespace intel
namespace detail {
namespace half_impl {
class half;
Expand Down Expand Up @@ -302,6 +306,20 @@ template <access::address_space AS, class DataT>
using const_if_const_AS = DataT;
#endif

template <typename T> struct is_group : std::false_type {};

template <int Dimensions>
struct is_group<group<Dimensions>> : std::true_type {};

template <typename T> struct is_sub_group : std::false_type {};

template <> struct is_sub_group<intel::sub_group> : std::true_type {};

template <typename T>
struct is_generic_group
: std::integral_constant<bool,
is_group<T>::value || is_sub_group<T>::value> {};

} // namespace detail
} // namespace sycl
} // __SYCL_INLINE_NAMESPACE(cl)
14 changes: 0 additions & 14 deletions sycl/include/CL/sycl/intel/group_algorithm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,20 +77,6 @@ template <> inline id<3> linear_id_to_id(range<3> r, size_t linear_id) {
return result;
}

template <typename T> struct is_group : std::false_type {};

template <int Dimensions>
struct is_group<group<Dimensions>> : std::true_type {};

template <typename T> struct is_sub_group : std::false_type {};

template <> struct is_sub_group<intel::sub_group> : std::true_type {};

template <typename T>
struct is_generic_group
: std::integral_constant<bool,
is_group<T>::value || is_sub_group<T>::value> {};

template <typename T, class BinaryOperation> struct identity {};

template <typename T, typename V> struct identity<T, intel::plus<V>> {
Expand Down