[Openmp-commits] [PATCH] D31205: [OpenMP] Allow multiple weak symbols to be loaded from the fat binary

[George Rokos via Phabricator via Openmp-commits]

grokos created this revision.
grokos added a project: OpenMP.
Herald added a subscriber: rengolin.

In Fortran, when linking multiple object files containing declarations to external symbols, the linker does not consolidate all weak declarations into one; instead, it allows multiple weak symbols to refer to the same external symbol. This causes problem when the fat binary is loaded from libomptarget, as the initial assumption was that a static symbol could only be loaded once, i.e. it would indicate some error if the fat binary contained the same symbol twice. This assumption does not hold true for Fortran executables, so for the sake of compatibility we need to allow the library to map multiple weak symbols.


Repository:
  rL LLVM

https://reviews.llvm.org/D31205

Files:
  libomptarget/src/omptarget.cpp


Index: libomptarget/src/omptarget.cpp
===================================================================
--- libomptarget/src/omptarget.cpp
+++ libomptarget/src/omptarget.cpp
@@ -1303,7 +1303,6 @@
     }
 
     // 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,
@@ -1314,20 +1313,20 @@
         // has data.
         assert(CurrDeviceEntry->size == CurrHostEntry->size &&
                "data size mismatch");
-        assert(Device.getTgtPtrBegin(CurrHostEntry->addr,
-                                     CurrHostEntry->size) == NULL &&
-               "data in declared target should not be already mapped");
-        // add entry to map.
+
+        // 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.
         DP("Add mapping from host " DPxMOD " to device " DPxMOD " with size %zu"
             "\n", DPxPTR(CurrHostEntry->addr), DPxPTR(CurrDeviceEntry->addr),
             CurrDeviceEntry->size);
-        Device.HostDataToTargetMap.push_front(HostDataToTargetTy(
-            (uintptr_t)CurrHostEntry->addr, (uintptr_t)CurrHostEntry->addr,
-            (uintptr_t)CurrHostEntry->addr + CurrHostEntry->size,
-            (uintptr_t)CurrDeviceEntry->addr));
+        bool IsNew; //unused
+        Device.getOrAllocTgtPtr(CurrHostEntry->addr /*HstPtrBegin*/,
+            CurrHostEntry->addr /*HstPtrBase*/, CurrHostEntry->size /*Size*/,
+            IsNew, false /*IsImplicit*/, true /*UpdateRefCount*/);
       }
     }
-    Device.DataMapMtx.unlock();
   }
   TrlTblMtx.unlock();
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31205.92533.patch
Type: text/x-patch
Size: 1934 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20170321/015513aa/attachment.bin>