[Openmp-commits] [PATCH] D73076: [libomptarget] Implement hip atomic functions in terms of intrinsics
Jon Chesterfield via Phabricator via Openmp-commits
openmp-commits at lists.llvm.org
Tue Jan 21 15:37:06 PST 2020
JonChesterfield added a comment.
In D73076#1832492 <https://reviews.llvm.org/D73076#1832492>, @jdoerfert wrote:
> Is is necessary, or useful, to have 3 different kinds of intrinsics here? We have `__atomic_fetch_max` and `__atomic_max_fetch` in clang, unsure if they could replace the OpenCL intrinsic. Similarly, there is `__atomic_compare_exchange` and `__atomic_exchange`. I would also be fine with only __amdgcn intrinsics or only __opencl ones but I am a little worried about mixing these 3 kinds.
I share this concern. I especially dislike the forced cast to _Atomic T* for the opencl intrinsics as that's skirting a bit close to an aliasing violation.
atomic_fetch_max I missed because gcc doesn't document it. In clang, it only compiles for 32 bit integer types (and we have a use of 64 bit in loop.cu). Some context in D46386 <https://reviews.llvm.org/D46386>.
I remember seeing different codegen for the opencl compare_exchange and the builtin but am not able to reproduce that now. There was some trial and error choosing intrinsics while comparing the codegen to hand written IR.
How about generic intrinsics where available:
Amdgcn prefixed IR functions for:
with the intent to provide amdgcn intrinsics for inc and fetch_max?
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the Openmp-commits