[Openmp-commits] [PATCH] D44260: [OpenMP][libomptarget] Add global memory data sharing support for master-worker sharing.

[Gheorghe-Teodor Bercea via Phabricator via Openmp-commits]

gtbercea added inline comments.


================
Comment at: libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.h:99-100
+  void **args;
+  // starts off as MAX_SHARED_ARGS but can increase in size.
+  uint32_t nArgs;
+};
----------------
grokos wrote:
> gtbercea wrote:
> > grokos wrote:
> > > `Init()` is never called throughout the code, so `nArgs` is never initialized to `MAX_SHARED_ARGS`. Add a constructor to this class which will call `Init()`.
> > Added the call in the stack initialization function.
> No, that's not a good idea. Initialization of the class members should be handled automatically via a constructor. If someone ever wants to reuse this class without having a constructor then they will need to keep in mind that `Init()` must be called explicitly - not a good style.
Since this class is going to be instantiated as a __device__ __shared__ object, dynamic initialization is not supported. In general, it is not supported for __device__, __constant__ and __shared__ variables. The suggested solution is to have an empty or no constructor at all and explicitly handle initialization like it is being done in the Init() method.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D44260