[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:

- fetch_add
- exchange
- compare_exchange

Amdgcn prefixed IR functions for:

- atomic_inc
- atomic_fetch_max

with the intent to provide amdgcn intrinsics for inc and fetch_max?

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list