[Openmp-commits] [PATCH] D32326: [OpenMP] libomptarget: Set ref count for global objects to positive infinity

George Rokos via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Sat Apr 22 04:30:25 PDT 2017


grokos added inline comments.


================
Comment at: libomptarget/src/omptarget.cpp:1326-1327
         // Add entry to map.
+        if (!Device.getTgtPtrBegin(CurrHostEntry->addr, CurrHostEntry->size))
+          continue;
         DP("Add mapping from host " DPxMOD " to device " DPxMOD " with size %zu"
----------------
Hahnfeld wrote:
> Shouldn't this be the other way round? If we already added this mapping before, just `continue`?
Right, I overlooked this one...


================
Comment at: libomptarget/src/omptarget.cpp:1331-1334
+        Device.HostDataToTargetMap.push_front(HostDataToTargetTy(
+            (uintptr_t)CurrHostEntry->addr, (uintptr_t)CurrHostEntry->addr,
+            (uintptr_t)CurrHostEntry->addr + CurrHostEntry->size,
+            (uintptr_t)CurrDeviceEntry->addr, INF_REF_CNT));
----------------
Hahnfeld wrote:
> 1. Are the casts neccessary?
> 2. Please add a comment that the first `addr` is `HostPtrBase` while the second is `HostPtrBegin`
One of the casts is necessary. `CurrHostEntry->addr` is of type `void *`. In order to add to it the size of the entry (`CurrHostEntry->addr + CurrHostEntry->size`) it must be cast to an integral type. The other casts are optional, but it helps make clear that the entries of `struct HostDataToTargetTy` are of type `uintptr_t`.


Repository:
  rL LLVM

https://reviews.llvm.org/D32326





More information about the Openmp-commits mailing list