[Openmp-commits] [PATCH] D81054: [OpenMP] Introduce target memory manager

Shilei Tian via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Tue Aug 18 16:45:38 PDT 2020

tianshilei1992 marked 3 inline comments as done.
tianshilei1992 added inline comments.

Comment at: openmp/libomptarget/src/device.cpp:31
+      DataMapMtx(), PendingGlobalsMtx(), ShadowMtx(),
+      LoopTripCnt(D.LoopTripCnt), MemoryManager(nullptr) {}
JonChesterfield wrote:
> tianshilei1992 wrote:
> > JonChesterfield wrote:
> > > tianshilei1992 wrote:
> > > > ye-luo wrote:
> > > > > Why do you think it is OK here leaving the copy constructor always setting MemoryManager nullptr? This cause surprises. The same question applies to assign operator as well.
> > > > `MemoryManager` will be initialized separately later. The only reason we need this is `std::vector<DeviceTy>` requires it. We don't copy or construct those objects afterwards.
> > > std::vector<DeviceTy> should be content with a move constructor. Then the copy constructor can be = delete.
> > `std::mutex` cannot be moved. That is the only reason we have the copy constructor.
> `std::mutex` can't be copied either. If a new default-initialised mutex is OK as the result of the copy, it would be OK as the result of a move too.
That is beyond the scope of this patch. Since it already has a user-defined copy operator, then let it be.

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list