[Openmp-commits] [PATCH] D64217: [OpenMP][NFCI] Cleanup the target state queue implementation

Alexey Bataev via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Thu Jul 4 15:42:30 PDT 2019


ABataev added inline comments.


================
Comment at: openmp/libomptarget/deviceRTLs/nvptx/src/state-queue.h:1
-//===--------- statequeue.h - NVPTX OpenMP GPU State Queue ------- CUDA -*-===//
+//===--- state-queue.h --- OpenMP target state queue ------------*- C++ -*-===//
 //
----------------
I think it is better to mark it as Cuda, not C/C++, since it contains some Cuda specific constructs.


================
Comment at: openmp/libomptarget/deviceRTLs/nvptx/src/state-queue.h:26
 
-template <typename ElementType, uint32_t SIZE> class omptarget_nvptx_Queue {
-private:
-  ElementType elements[SIZE];
-  volatile ElementType *elementQueue[SIZE];
-  volatile uint32_t head;
-  volatile uint32_t ids[SIZE];
-  volatile uint32_t tail;
+template <typename ElementType, uint32_t SIZE> class omptarget_state_queue {
+  ElementType Elements[SIZE];
----------------
Maybe it is better to name it in LLVM style, without underscores etc.?


================
Comment at: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h:19
+/// value of the pointee.
+template <typename T> T __kmpc_impl_atomic_add(T *Ptr, T Val) {
+  return atomicAdd(Ptr, Val);
----------------
Why do we need these functions? IMHO, they do not add anything useful, just increase code complexity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64217





More information about the Openmp-commits mailing list