Cap vector size to kernel maximum for SYCL #4704
Conversation
|
@masterleinad So from the discussion, this does work for all parallel_fors in KokkosKernels which are valid Kokkos. |
| std::stringstream out; | ||
| out << "The maximum subgroup size (" << max_sg_size | ||
| << ") for this kernel is not divisible by the vector_size (" | ||
| << m_vector_size << "). Choose a smaller vector_size!\n"; | ||
| << final_vector_size << "). Choose a smaller vector_size!\n"; |
There was a problem hiding this comment.
The error message needs to be revisited
There was a problem hiding this comment.
Can you elaborate on why this needs to be revisited? Should we explain that the vector size might have been capped?
There was a problem hiding this comment.
The error message will only be triggered if m_vector_size was smaller than max_sg_size in the first place (since you use the min). So one doesn't need to necessarily choose a smaller vector size, just something which is divisible (which could be larger). That said, how would this be ever triggered? Is max_sg_size not a power of two? We kinda have that requirement for vector length in team policy.
| std::stringstream out; | ||
| out << "The maximum subgroup size (" << max_sg_size | ||
| << ") for this kernel is not divisible by the vector_size (" | ||
| << m_vector_size << "). Choose a smaller vector_size!\n"; | ||
| << final_vector_size << "). Choose a smaller vector_size!\n"; |
There was a problem hiding this comment.
The error message will only be triggered if m_vector_size was smaller than max_sg_size in the first place (since you use the min). So one doesn't need to necessarily choose a smaller vector size, just something which is divisible (which could be larger). That said, how would this be ever triggered? Is max_sg_size not a power of two? We kinda have that requirement for vector length in team policy.
| std::stringstream out; | ||
| out << "The maximum subgroup size (" << max_sg_size | ||
| << ") for this kernel is not divisible by the vector_size (" | ||
| << m_vector_size << "). Choose a smaller vector_size!\n"; | ||
| << final_vector_size << "). Choose a smaller vector_size!\n"; |
There was a problem hiding this comment.
Yes, the used vector length is the largest power-of-two not exceeding the input vector length. The SYCL standard doesn't specify that the possible subgroup sizes are powers-of-two but that at least seems to be true for NVIDIA and Intel GPUs.
I think we can remove the check.
|
Failure (compiler crashing in OpenMPTarget build on Nvidia GPU) is clearly unrelated |
Fixes #4573.
For SYCL, the maximum vector size depends on the kernel and the maximum reported by the device might not be supported for every kernel. So far, we just errored out in case we detected the vector size being too large. This pull request tries to cap the vector size to the maximum supported by the kernel instead.
This is easy enough for
parallel_forsince the kernel doesn't use the vector size explicitly (meaning that it's fine to retrieve from within the kernel) but forparallel_reduceallocations for the reductions actually depend on the total workgroup size.The idea here is to first build a kernel with dummy pointers for reduction results and temporary storage, use that one for querying the maximum vector size for the kernel, allocating temporary global and local space, and create the final kernel to be launched with these pointers.
The actual changes are not that large if you ignore whitespace changes.