[Openmp-commits] [PATCH] D60223: [OpenMP][libomptarget] Add support for target link variables when unified memory is enabled

Alexandre Eichenberger via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Wed Apr 3 12:23:37 PDT 2019

AlexEichenberger requested changes to this revision.
AlexEichenberger marked 2 inline comments as done.
AlexEichenberger added a comment.
This revision now requires changes to proceed.

see comments

Comment at: libomptarget/include/omptarget.h:51
+  /// Target link
   // member of struct, member given by [16 MSBs] - 1
ABataev wrote:
> Do you really need this flag? Can you use something different? For example, `OMP_TGT_MAPTYPE_LITERAL` or `OMP_TGT_MAPTYPE_PRIVATE`?
I don't believe it is needed, as all is handled while registering the device library. If anything, you could just use the literal to naturally move the host pointer address to the device. This is if you want to avoid going through the indirection already set in place for globals mapped with "link" clause

Comment at: libomptarget/plugins/cuda/src/rtl.cpp:443
+        // be overwritten.
+        cuMemcpyHtoD(cuptr, e->addr, 8);
+        DP("Copy linked variable host address (" DPxMOD ")"
ABataev wrote:
> `8`->`sizeof(void*)`
Shouldn't this code be guarded by some "if the runtime is in unified memory mode,"  and if we see TARGET_LINK and the runtime is not in unified memory mode, should't there be some error as things are likely to go seriously wrong? 

Comment at: libomptarget/src/rtl.cpp:180
-      if (entry->flags & OMP_DECLARE_TARGET_LINK) {
Same comment here... if the runtime is in non-unified memory mode, should't it be an error to see a link?

  rOMP OpenMP



More information about the Openmp-commits mailing list