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

Dhruva Chakrabarti via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Thu Jun 29 18:32:03 PDT 2023

dhruvachak added inline comments.

Comment at: openmp/libomptarget/DeviceRTL/include/Synchronization.h:29
+enum MemScopeTy {
+  all,    // All threads on all devices
arsenm wrote:
> Why does this need a new subset abstraction for scopes? I could understand a complete enum around all the names target scopes 
I tried to keep the memscope arch-independent, hence the additional abstraction. I kept the same memscopes as what the upcoming OpenMP spec is going to have. This way, the call to atomic::inc in DeviceRTL (e.g. in reduction) does not have to be arch-dependent. In other words, I did not want to pass in "agent" directly from the reduction code for amdgpu, as an example.

Comment at: openmp/libomptarget/DeviceRTL/src/Reduction.cpp:226
+#ifndef __AMDGCN__
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.

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list