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

[Matt Arsenault via Phabricator via Openmp-commits]

arsenm added inline comments.


================
Comment at: openmp/libomptarget/DeviceRTL/include/Synchronization.h:29
 
+enum MemScopeTy {
+  all,    // All threads on all devices
----------------
Why does this need a new subset abstraction for scopes? I could understand a complete enum around all the names target scopes 


================
Comment at: openmp/libomptarget/DeviceRTL/src/Reduction.cpp:226
 
+#ifndef __AMDGCN__
     fence::system(atomic::seq_cst);
----------------
I doubt this is target specific 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154172