[OpenMP][libomptarget] Delay restore of shadow pointers in structs to after H2D memory copies are completed

[carlo.bertolli]

Summary

When using asynchronous plugin calls, shadow pointer restore could happen before the D2H copy for the entire struct has completed, effectively leaving a device pointer in a host struct. This patch fixes the problem by delaying restore's to after a synchronization happens (target regions) and by calling early synchronization (target update).

[jdoerfert]

openmp/libomptarget/src/omptarget.cpp

897

Why don't we do this above as well. Seems much simpler and the delay we buy us with the complexity is minimal anyway.

[carlo.bertolli]

openmp/libomptarget/src/omptarget.cpp

897

Agreed: good short term solution and keeps complexity down. I will update the patch asap.

[carlo.bertolli]

Replace list of shadow pointer to be restored with simple synchronization.

[jdoerfert]

Nits: Wait (capital W) and "." at the end. Both comments should be proper sentences.

LG

openmp/libomptarget/src/omptarget.cpp

if ((ArgTypes[I] & OMP_TGT_MAPTYPE_FROM) || DelEntry) {
       // If we copied back to the host a struct/array containing pointers, we
       // need to restore the original host pointer values from their shadow
       // copies. If the struct is going to be deallocated, remove any remaining
       // shadow pointer entries for this struct.
       auto CB = [&](ShadowPtrListTy::iterator &Itr) {
         // If we copied the struct to the host, we need to restore the pointer.
         if (ArgTypes[I] & OMP_TGT_MAPTYPE_FROM) {
           void **ShadowHstPtrAddr = (void **)Itr->first;
+          // Wait for device-to-host memcopies for whole struct to complete,
+          // before restoring the correct host pointer.
+          if (AsyncInfo.synchronize() != OFFLOAD_SUCCESS)
+            return OFFLOAD_FAIL;
           *ShadowHstPtrAddr = Itr->second.HstPtrVal;
           DP("Restoring original host pointer value " DPxMOD " for host "
              "pointer " DPxMOD "\n",
              DPxPTR(Itr->second.HstPtrVal), DPxPTR(ShadowHstPtrAddr));
         }
         // If the struct is to be deallocated, remove the shadow entry.
         if (DelEntry) {
           DP("Removing shadow pointer " DPxMOD "\n",
if (ArgType & OMP_TGT_MAPTYPE_FROM) {
     int Ret = Device.retrieveData(HstPtrBegin, TgtPtrBegin, ArgSize, AsyncInfo);
     if (Ret != OFFLOAD_SUCCESS) {
       REPORT("Copying data from device failed.\n");
       return OFFLOAD_FAIL;
     }
 
     auto CB = [&](ShadowPtrListTy::iterator &Itr) {
       void **ShadowHstPtrAddr = (void **)Itr->first;
+      // Wait for device-to-host memcopies for whole struct to complete,
+      // before restoring the correct host pointer.
+      if (AsyncInfo.synchronize() != OFFLOAD_SUCCESS)
+        return OFFLOAD_FAIL;
       *ShadowHstPtrAddr = Itr->second.HstPtrVal;
       DP("Restoring original host pointer value " DPxMOD
          " for host pointer " DPxMOD "\n",
          DPxPTR(Itr->second.HstPtrVal), DPxPTR(ShadowHstPtrAddr));
       ++Itr;
       return OFFLOAD_SUCCESS;
     };
     applyToShadowMapEntries(Device, CB, HstPtrBegin, ArgSize, TPR);
   }