[PATCH] D82087: AMDGPU/clang: Add builtins for llvm.amdgcn.ballot

[Sameer Sahasrabuddhe via Phabricator via cfe-commits]

sameerds added a comment.

>> https://github.com/ROCm-Developer-Tools/HIP/blob/master/docs/markdown/hip_kernel_language.md#warp-vote-and-ballot-functions
> 
> I think if the language interface insists on fixing the wave size, then I think the correct solution is to implement this in the header based on a wave size macro (which we're desperately missing). The library implementation should be responsible for inserting the extension to 64-bit for wave32

Not sure if the frontend should try to infer warpsize and the mask size, or even whether it can in all cases. But this can result in wrong behaviour when the program passes 32-bit mask but then gets compiled for a 64-bit mask. It's easy to say that the programmer must not assume a warp-size, but it would be useful if the language can

In D82087#2144712 <https://reviews.llvm.org/D82087#2144712>, @arsenm wrote:

> In D82087#2140778 <https://reviews.llvm.org/D82087#2140778>, @sameerds wrote:
>
> > The documentation for HIP __ballot seems to indicate that the user does not have to explicitly specify the warp size. How is that achieved with these new builtins? Can this  be captured in a lit test here?
>
>
> This seems like a defect in the design to me
>
> > https://github.com/ROCm-Developer-Tools/HIP/blob/master/docs/markdown/hip_kernel_language.md#warp-vote-and-ballot-functions


I tend to agree. The HIP vote/ballot builtins are also missing a mask parameter, whose type needs to match the wavesize.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82087/new/

https://reviews.llvm.org/D82087