[SYCL] Infer IdT in GroupBroadcast from group type #2115
Conversation
- Work-group => any integral type - Sub-group => unsigned 32-bit integer Signed-off-by: John Pennycook <[email protected]>
Signed-off-by: John Pennycook <[email protected]>
| 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, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
It's broadcast.cpp, but you won't see it in CI results as it fails on OpenCL CPU device, which wasn't tested due to failures on Level Zero back-end. |
Change `auto` to `auto *` when the type is a pointer. This makes the code comply with the clang-tidy `llvm-qualified-auto` check. That check is already enabled, but the code base wasn't fully compliant yet. Original commit: KhronosGroup/SPIRV-LLVM-Translator@aa9226e
Signed-off-by: John Pennycook [email protected]
Fixes failing broadcast test in #2049.