[Openmp-commits] [PATCH] D154172: [OpenMP] Added memory scope to atomic::inc API and used the device scope in reduction.

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added inline comments.


================
Comment at: openmp/libomptarget/DeviceRTL/src/Reduction.cpp:226
 
+#ifndef __AMDGCN__
     fence::system(atomic::seq_cst);
----------------
dhruvachak wrote:
> arsenm wrote:
> > I doubt this is target specific 
> Well, I don't know whether it is required for NVPTX for example, so I kept it around.
> 
> As an aside, I think the system fence is too conservative, but again I kept it unchanged for archs other than amdgpu.
> As an aside, I think the system fence is too conservative, but again I kept it unchanged for archs other than amdgpu.

1) This is unrelated.
2) Could you elaborate on why you think the change is sound (on AMDGPUs)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154172