diff --git a/openmp/libomptarget/src/device.h b/openmp/libomptarget/src/device.h --- a/openmp/libomptarget/src/device.h +++ b/openmp/libomptarget/src/device.h @@ -49,6 +49,9 @@ uintptr_t TgtPtrBegin; // target info. + /// Pointer to the event corresponding to the data update of this map. + mutable void *Event; + private: /// use mutable to allow modification via std::set iterator which is const. mutable uint64_t RefCount; @@ -65,7 +68,7 @@ HostDataToTargetTy(uintptr_t BP, uintptr_t B, uintptr_t E, uintptr_t TB, map_var_info_t Name = nullptr, bool IsINF = false) : HstPtrBase(BP), HstPtrBegin(B), HstPtrEnd(E), HstPtrName(Name), - TgtPtrBegin(TB), RefCount(IsINF ? INFRefCount : 1), + TgtPtrBegin(TB), Event(nullptr), RefCount(IsINF ? INFRefCount : 1), UpdateMtx(std::make_shared()) {} uint64_t getRefCount() const { return RefCount; } @@ -228,8 +231,8 @@ void *getTgtPtrBegin(void *HstPtrBegin, int64_t Size, bool &IsLast, bool UpdateRefCount, bool &IsHostPtr, bool MustContain = false, bool ForceDelete = false); - int deallocTgtPtr(void *TgtPtrBegin, int64_t Size, - bool HasCloseModifier = false); + int deallocTgtPtr(void *TgtPtrBegin, int64_t Size, bool HasCloseModifier, + AsyncInfoTy &AsyncInfo); int associatePtr(void *HstPtrBegin, void *TgtPtrBegin, int64_t Size); int disassociatePtr(void *HstPtrBegin); diff --git a/openmp/libomptarget/src/device.cpp b/openmp/libomptarget/src/device.cpp --- a/openmp/libomptarget/src/device.cpp +++ b/openmp/libomptarget/src/device.cpp @@ -105,8 +105,11 @@ // Mapping exists if (search->isRefCountInf()) { DP("Association found, removing it\n"); + void *Event = search->Event; HostDataToTargetMap.erase(search); DataMapMtx.unlock(); + if (Event) + destroyEvent(Event); return OFFLOAD_SUCCESS; } else { REPORT("Trying to disassociate a pointer which was not mapped via " @@ -259,29 +262,82 @@ if (IsNew && MoveData == MoveDataStateTy::UNKNOWN) MoveData = MoveDataStateTy::REQUIRED; - // If the target pointer is valid, and we need to transfer data, issue the - // data transfer. - if (TargetPointer && (MoveData == MoveDataStateTy::REQUIRED)) { - // Lock the entry before releasing the mapping table lock such that another - // thread that could issue data movement will get the right result. - Entry->lock(); - // Release the mapping table lock right after the entry is locked. - DataMapMtx.unlock(); + if (TargetPointer) { + // If the target pointer is valid, and we need to transfer data, issue the + // data transfer. + if (MoveData == MoveDataStateTy::REQUIRED) { + // Lock the entry before releasing the mapping table lock such that + // another thread that could issue data movement will get the right + // result. + Entry->lock(); + // Release the mapping table lock right after the entry is locked. + DataMapMtx.unlock(); - DP("Moving %" PRId64 " bytes (hst:" DPxMOD ") -> (tgt:" DPxMOD ")\n", Size, - DPxPTR(HstPtrBegin), DPxPTR(TargetPointer)); + DP("Moving %" PRId64 " bytes (hst:" DPxMOD ") -> (tgt:" DPxMOD ")\n", + Size, DPxPTR(HstPtrBegin), DPxPTR(TargetPointer)); - int Ret = submitData(TargetPointer, HstPtrBegin, Size, AsyncInfo); + int Ret = submitData(TargetPointer, HstPtrBegin, Size, AsyncInfo); - // Unlock the entry immediately after the data movement is issued. - Entry->unlock(); + if (Ret != OFFLOAD_SUCCESS) { + // Unlock the entry immediately if data movement issuing reports error. + Entry->unlock(); - if (Ret != OFFLOAD_SUCCESS) { - REPORT("Copying data to device failed.\n"); - // We will also return nullptr if the data movement fails because that - // pointer points to a corrupted memory region so it doesn't make any - // sense to continue to use it. - TargetPointer = nullptr; + REPORT("Copying data to device failed.\n"); + // We will also return nullptr if the data movement fails because that + // pointer points to a corrupted memory region so it doesn't make any + // sense to continue to use it. + TargetPointer = nullptr; + } + + // Create an event at this moment and attach it to the entry. + void *Event = nullptr; + Ret = createEvent(&Event); + if (Ret != OFFLOAD_SUCCESS) { + Entry->unlock(); + REPORT("Failed to create event\n"); + return {{false /* IsNewEntry */, false /* IsHostPointer */}, + {} /* MapTableEntry */, + nullptr /* TargetPointer */}; + } + // We cannot assume the event should not be nullptr because we don't + // know if the target support event. But if a target doesn't, + // recordEvent should always return success. + Ret = recordEvent(Event, AsyncInfo); + if (Ret != OFFLOAD_SUCCESS) { + Entry->unlock(); + REPORT("Failed to set dependence on event " DPxMOD "\n", DPxPTR(Event)); + return {{false /* IsNewEntry */, false /* IsHostPointer */}, + {} /* MapTableEntry */, + nullptr /* TargetPointer */}; + } + void *OldEvent = Entry->Event; + Entry->Event = Event; + // We're done with the entry. Release the entry. + Entry->unlock(); + // If there is an event attached, destroy it. + if (OldEvent) + destroyEvent(OldEvent); + } else { + // Release the mapping table lock directly. + DataMapMtx.unlock(); + // If not a host pointer, we need to wait for the event if it exists. + if (!IsHostPtr) { + Entry->lock(); + void *Event = Entry->Event; + Entry->unlock(); + + if (Event) { + int Ret = waitEvent(Event, AsyncInfo); + if (Ret != OFFLOAD_SUCCESS) { + // If it fails to wait for the event, we need to return nullptr in + // case of any data race. + REPORT("Failed to wait for event " DPxMOD ".\n", DPxPTR(Event)); + return {{false /* IsNewEntry */, false /* IsHostPointer */}, + {} /* MapTableEntry */, + nullptr /* TargetPointer */}; + } + } + } } } else { // Release the mapping table lock directly. @@ -366,7 +422,7 @@ } int DeviceTy::deallocTgtPtr(void *HstPtrBegin, int64_t Size, - bool HasCloseModifier) { + bool HasCloseModifier, AsyncInfoTy &AsyncInfo) { if (PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY && !HasCloseModifier) return OFFLOAD_SUCCESS; @@ -386,7 +442,10 @@ DPxPTR(HT.HstPtrBegin), DPxPTR(HT.TgtPtrBegin), Size, (HT.HstPtrName) ? getNameFromMapping(HT.HstPtrName).c_str() : "unknown"); + void *Event = lr.Entry->Event; HostDataToTargetMap.erase(lr.Entry); + if (Event) + destroyEvent(Event); } rc = OFFLOAD_SUCCESS; } else { diff --git a/openmp/libomptarget/src/omptarget.cpp b/openmp/libomptarget/src/omptarget.cpp --- a/openmp/libomptarget/src/omptarget.cpp +++ b/openmp/libomptarget/src/omptarget.cpp @@ -582,14 +582,38 @@ void *&TgtPtrBase = AsyncInfo.getVoidPtrLocation(); TgtPtrBase = ExpectedTgtPtrBase; - int rt = Device.submitData(PointerTgtPtrBegin, &TgtPtrBase, - sizeof(void *), AsyncInfo); - Pointer_TPR.MapTableEntry->unlock(); - - if (rt != OFFLOAD_SUCCESS) { + int Ret = Device.submitData(PointerTgtPtrBegin, &TgtPtrBase, + sizeof(void *), AsyncInfo); + if (Ret != OFFLOAD_SUCCESS) { + Pointer_TPR.MapTableEntry->unlock(); REPORT("Copying data to device failed.\n"); return OFFLOAD_FAIL; } + // Create a new event. + void *Event = nullptr; + Ret = Device.createEvent(&Event); + if (Ret != OFFLOAD_SUCCESS) { + Pointer_TPR.MapTableEntry->unlock(); + REPORT("Failed to create event\n"); + return OFFLOAD_FAIL; + } + // We cannot assume the event should not be nullptr because we don't + // know if the target support event. But if a target doesn't, + // recordEvent should always return success. + Ret = Device.recordEvent(Event, AsyncInfo); + if (Ret != OFFLOAD_SUCCESS) { + Pointer_TPR.MapTableEntry->unlock(); + REPORT("Failed to set dependence on event " DPxMOD "\n", + DPxPTR(Event)); + return OFFLOAD_FAIL; + } + // Exchange the old event with new created event + void *OldEvent = Pointer_TPR.MapTableEntry->Event; + Pointer_TPR.MapTableEntry->Event = Event; + Pointer_TPR.MapTableEntry->unlock(); + // If the old event is not null, we need to destroy it. + if (OldEvent) + Device.destroyEvent(OldEvent); } else Device.ShadowMtx.unlock(); } @@ -816,7 +840,7 @@ if (FromMapperBase && FromMapperBase == Info.HstPtrBegin) continue; Ret = Device.deallocTgtPtr(Info.HstPtrBegin, Info.DataSize, - Info.HasCloseModifier); + Info.HasCloseModifier, AsyncInfo); if (Ret != OFFLOAD_SUCCESS) { REPORT("Deallocating data from device failed.\n"); return OFFLOAD_FAIL;