[OpenMP] libomptarget: Set ref count for global objects to positive infinity

[grokos]

Summary

According to the standard, the reference counter of global objects should be positive infinity. Right now it is set to 1.

[Hahnfeld]

libomptarget/src/omptarget.cpp

1326–1327 ↗

(On Diff #96053)

Shouldn't this be the other way round? If we already added this mapping before, just continue?

1331–1334 ↗

(On Diff #96053)

1. Are the casts neccessary?
2. Please add a comment that the first addr is HostPtrBase while the second is HostPtrBegin

[grokos]

Fixed bug.

libomptarget/src/omptarget.cpp

1326–1327 ↗

(On Diff #96053)

Right, I overlooked this one...

1331–1334 ↗

(On Diff #96053)

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.

[Hahnfeld]

LGTM

openmp/trunk/libomptarget/src/omptarget.cpp

struct HostDataToTargetTy {
   long RefCount;
 
   HostDataToTargetTy()
       : HstPtrBase(0), HstPtrBegin(0), HstPtrEnd(0),
         TgtPtrBegin(0), RefCount(0) {}
   HostDataToTargetTy(uintptr_t BP, uintptr_t B, uintptr_t E, uintptr_t TB)
       : HstPtrBase(BP), HstPtrBegin(B), HstPtrEnd(E),
         TgtPtrBegin(TB), RefCount(1) {}
+  HostDataToTargetTy(uintptr_t BP, uintptr_t B, uintptr_t E, uintptr_t TB,
+      long RF)
+      : HstPtrBase(BP), HstPtrBegin(B), HstPtrEnd(E),
+        TgtPtrBegin(TB), RefCount(RF) {}
 };
 
 typedef std::list<HostDataToTargetTy> HostDataToTargetListTy;
 
 struct LookupResult {
   struct {
     unsigned IsContained   : 1;
     unsigned ExtendsBefore : 1;
if (lr.Flags.IsContained || lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter) {
     IsLast = false;
   }
 
   DataMapMtx.unlock();
   return rc;
 }
 
 // Return the target pointer begin (where the data will be moved).
-// Lock-free version called from within assertions.
+// Lock-free version called when loading global symbols from the fat binary.
 void *DeviceTy::getTgtPtrBegin(void *HstPtrBegin, int64_t Size) {
   uintptr_t hp = (uintptr_t)HstPtrBegin;
   LookupResult lr = lookupMapping(HstPtrBegin, Size);
   if (lr.Flags.IsContained || lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter) {
     auto &HT = *lr.Entry;
     uintptr_t tp = HT.TgtPtrBegin + (hp - HT.HstPtrBegin);
     return (void *)tp;
   }
if (hsize != tsize) {
          device_id, hsize, tsize);
       TransTable->TargetsImages[device_id] = 0;
       TransTable->TargetsTable[device_id] = 0;
       rc = OFFLOAD_FAIL;
       break;
     }
 
     // process global data that needs to be mapped.
+    Device.DataMapMtx.lock();
     __tgt_target_table *HostTable = &TransTable->HostTable;
     for (__tgt_offload_entry *CurrDeviceEntry = TargetTable->EntriesBegin,
                              *CurrHostEntry = HostTable->EntriesBegin,
                              *EntryDeviceEnd = TargetTable->EntriesEnd;
          CurrDeviceEntry != EntryDeviceEnd;
          CurrDeviceEntry++, CurrHostEntry++) {
       if (CurrDeviceEntry->size != 0) {
         // has data.
         assert(CurrDeviceEntry->size == CurrHostEntry->size &&
                "data size mismatch");
 
         // Fortran may use multiple weak declarations for the same symbol,
         // therefore we must allow for multiple weak symbols to be loaded from
         // the fat binary. Treat these mappings as any other "regular" mapping.
         // Add entry to map.
+        if (Device.getTgtPtrBegin(CurrHostEntry->addr, CurrHostEntry->size))
+          continue;
         DP("Add mapping from host " DPxMOD " to device " DPxMOD " with size %zu"
             "\n", DPxPTR(CurrHostEntry->addr), DPxPTR(CurrDeviceEntry->addr),
             CurrDeviceEntry->size);
-        bool IsNew; //unused
-        Device.getOrAllocTgtPtr(CurrHostEntry->addr /*HstPtrBegin*/,
-            CurrHostEntry->addr /*HstPtrBase*/, CurrHostEntry->size /*Size*/,
-            IsNew, false /*IsImplicit*/, true /*UpdateRefCount*/);
+        Device.HostDataToTargetMap.push_front(HostDataToTargetTy(
+            (uintptr_t)CurrHostEntry->addr /*HstPtrBase*/,
+            (uintptr_t)CurrHostEntry->addr /*HstPtrBegin*/,
+            (uintptr_t)CurrHostEntry->addr + CurrHostEntry->size /*HstPtrEnd*/,
+            (uintptr_t)CurrDeviceEntry->addr /*TgtPtrBegin*/,
+            INF_REF_CNT /*RefCount*/));
       }
     }
+    Device.DataMapMtx.unlock();
   }
   TrlTblMtx.unlock();
 
   if (rc != OFFLOAD_SUCCESS) {
     Device.PendingGlobalsMtx.unlock();
     return rc;
   }