[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 18:46:58 PST 2020
JonChesterfield added inline comments.
Comment at: openmp/libomptarget/deviceRTLs/amdgcn/src/atomic_inc.ll:12
+ %ret = call i32 @llvm.amdgcn.atomic.inc.i32.p0i32(i32* %x, i32 %v,
+ i32 5, ; Ordering. AtomicOrdering.h: sequentially consistent
> We should also have a builtin for this. It would not be difficult to add
Agreed and hopefully. It takes a lot of parameters so I was unsure about testing the codegen. Are you willing to review such a patch?
Comment at: openmp/libomptarget/deviceRTLs/amdgcn/src/atomic_inc.ll:15
+ i32 2, ; Scope. SyncScope.h: OpenCLAllSVMDevices is 2
+ i1 1 ; Volatile. True for consistency with other atomic operations
> This should be false. Where are you seeing this imply volatile? I think we started hacking around OpenCL volatile for atomics in device-libs
Fun story. We were using functions from hc_atomic.ll though a wrapper and those functions specified volatile. This does the same to preserve that behaviour.
However, dropping volatile from all of these functions doesn't actually break the build. Compiles about as cleanly as before, same set of tests pass. Therefore dropped the qualifier from all of these functions.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the Openmp-commits