diff --git a/openmp/libomptarget/include/device.h b/openmp/libomptarget/include/device.h
--- a/openmp/libomptarget/include/device.h
+++ b/openmp/libomptarget/include/device.h
@@ -61,7 +61,7 @@
struct StatesTy {
StatesTy(uint64_t DRC, uint64_t HRC)
: DynRefCount(DRC), HoldRefCount(HRC),
- MayContainAttachedPointers(false), DeleteThreadId(std::thread::id()) {
+ MayContainAttachedPointers(false) {
}
/// The dynamic reference count is the standard reference count as of OpenMP
/// 4.5. The hold reference count is an OpenMP extension for the sake of
@@ -100,14 +100,6 @@
/// mechanism for D2H, and if the event cannot be shared between them, Event
/// should be written as void *Event[2].
void *Event = nullptr;
-
- /// The id of the thread responsible for deleting this entry. This thread
- /// set the reference count to zero *last*. Other threads might reuse the
- /// entry while it is marked for deletion but not yet deleted (e.g., the
- /// data is still being moved back). If another thread reuses the entry we
- /// will have a non-zero reference count *or* the thread will have changed
- /// this id, effectively taking over deletion responsibility.
- std::thread::id DeleteThreadId;
};
// When HostDataToTargetTy is used by std::set, std::set::iterator is const
// use unique_ptr to make States mutable.
@@ -148,14 +140,6 @@
/// Returns OFFLOAD_FAIL if something went wrong, OFFLOAD_SUCCESS otherwise.
int addEventIfNecessary(DeviceTy &Device, AsyncInfoTy &AsyncInfo) const;
- /// Indicate that the current thread expected to delete this entry.
- void setDeleteThreadId() const {
- States->DeleteThreadId = std::this_thread::get_id();
- }
-
- /// Return the thread id of the thread expected to delete this entry.
- std::thread::id getDeleteThreadId() const { return States->DeleteThreadId; }
-
/// Set the event bound to this data map.
void setEvent(void *Event) const { States->Event = Event; }
@@ -281,7 +265,8 @@
unsigned IsNewEntry : 1;
/// If the pointer is actually a host pointer (when unified memory enabled)
unsigned IsHostPointer : 1;
- } Flags = {0, 0};
+ unsigned IsOwned : 1;
+ } Flags = {0, 0, 0};
/// The corresponding map table entry which is stable.
HostDataToTargetTy *Entry = nullptr;
@@ -388,7 +373,8 @@
/// not. It is the caller's responsibility to skip calling this function if
/// the map entry is not expected to exist because \p HstPtrBegin uses shared
/// memory.
- int deallocTgtPtr(HDTTMapAccessorTy &HDTTMap, LookupResult LR, int64_t Size);
+ int eraseHDTTMapEntry(HDTTMapAccessorTy &HDTTMap, LookupResult LR);
+ int deallocTgtPtr(HostDataToTargetTy *Entry, int64_t Size);
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
@@ -384,11 +384,6 @@
if (!UpdateRefCount) {
RefCountAction = " (update suppressed)";
} else if (IsLast) {
- // Mark the entry as to be deleted by this thread. Another thread might
- // reuse the entry and take "ownership" for the deletion while this thread
- // is waiting for data transfers. That is fine and the current thread will
- // simply skip the deletion step then.
- HT.setDeleteThreadId();
HT.decRefCount(UseHoldRefCount);
assert(HT.getTotalRefCount() == 0 &&
"Expected zero reference count when deletion is scheduled");
@@ -420,7 +415,11 @@
TargetPointer = HstPtrBegin;
}
- return {{IsNew, IsHostPtr}, LR.Entry, TargetPointer};
+ const bool TransferOwnership = IsLast && UpdateRefCount;
+ if (TransferOwnership)
+ eraseHDTTMapEntry(HDTTMap, LR);
+
+ return {{IsNew, IsHostPtr, TransferOwnership}, LR.Entry, TargetPointer};
}
// Return the target pointer begin (where the data will be moved).
@@ -437,8 +436,7 @@
return NULL;
}
-int DeviceTy::deallocTgtPtr(HDTTMapAccessorTy &HDTTMap, LookupResult LR,
- int64_t Size) {
+int DeviceTy::eraseHDTTMapEntry(HDTTMapAccessorTy &HDTTMap, LookupResult LR) {
// Check if the pointer is contained in any sub-nodes.
if (!(LR.Flags.IsContained || LR.Flags.ExtendsBefore ||
LR.Flags.ExtendsAfter)) {
@@ -451,9 +449,14 @@
auto &HT = *LR.Entry;
// Verify this thread is still in charge of deleting the entry.
assert(HT.getTotalRefCount() == 0 &&
- HT.getDeleteThreadId() == std::this_thread::get_id() &&
"Trying to delete entry that is in use or owned by another thread.");
+ HDTTMap->erase(LR.Entry);
+ return OFFLOAD_SUCCESS;
+}
+
+int DeviceTy::deallocTgtPtr(HostDataToTargetTy *Entry, int64_t Size) {
+ auto &HT = *Entry;
DP("Deleting tgt data " DPxMOD " of size %" PRId64 "\n",
DPxPTR(HT.TgtPtrBegin), Size);
deleteData((void *)HT.TgtPtrBegin);
@@ -462,9 +465,8 @@
", Size=%" PRId64 ", Name=%s\n",
DPxPTR(HT.HstPtrBegin), DPxPTR(HT.TgtPtrBegin), Size,
(HT.HstPtrName) ? getNameFromMapping(HT.HstPtrName).c_str() : "unknown");
- void *Event = LR.Entry->getEvent();
- HDTTMap->erase(LR.Entry);
- delete LR.Entry;
+ void *Event = Entry->getEvent();
+ delete Entry;
int Ret = OFFLOAD_SUCCESS;
if (Event && destroyEvent(Event) != OFFLOAD_SUCCESS) {
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
@@ -623,14 +623,10 @@
/// The target pointer information.
TargetPointerResultTy TPR;
- /// Are we expecting to delete this entry or not. Even if set, we might not
- /// delete the entry if another thread reused the entry in the meantime.
- bool DelEntry;
-
- PostProcessingInfo(void *HstPtr, int64_t Size, int64_t ArgType, bool DelEntry,
+ PostProcessingInfo(void *HstPtr, int64_t Size, int64_t ArgType,
TargetPointerResultTy TPR)
- : HstPtrBegin(HstPtr), DataSize(Size), ArgType(ArgType), TPR(TPR),
- DelEntry(DelEntry) {}
+ : HstPtrBegin(HstPtr), DataSize(Size), ArgType(ArgType), TPR(TPR)
+ {}
};
/// Apply \p CB to the shadow map pointer entries in the range \p Begin, to
@@ -682,7 +678,6 @@
void **ArgMappers, AsyncInfoTy &AsyncInfo, bool FromMapper) {
int Ret;
SmallVector PostProcessingPtrs;
- void *FromMapperBase = nullptr;
// process each input.
for (int32_t I = ArgNum - 1; I >= 0; --I) {
// Ignore private variables and arrays - there is no mapping for them.
@@ -828,14 +823,9 @@
}
}
}
- if (DelEntry && FromMapper && I == 0) {
- DelEntry = false;
- FromMapperBase = HstPtrBegin;
- }
// Add pointer to the buffer for post-synchronize processing.
- PostProcessingPtrs.emplace_back(HstPtrBegin, DataSize, ArgTypes[I],
- DelEntry && !IsHostPtr, TPR);
+ PostProcessingPtrs.emplace_back(HstPtrBegin, DataSize, ArgTypes[I], TPR);
}
}
@@ -849,26 +839,7 @@
// Deallocate target pointer
for (PostProcessingInfo &Info : PostProcessingPtrs) {
- // If we marked the entry to be deleted we need to verify no other thread
- // reused it by now. If deletion is still supposed to happen by this thread
- // LR will be set and exclusive access to the HDTT map will avoid another
- // thread reusing the entry now. Note that we do not request (exclusive)
- // access to the HDTT map if Info.DelEntry is not set.
- LookupResult LR;
- DeviceTy::HDTTMapAccessorTy HDTTMap =
- Device.HostDataToTargetMap.getExclusiveAccessor(!Info.DelEntry);
-
- if (Info.DelEntry) {
- LR = Device.lookupMapping(HDTTMap, Info.HstPtrBegin, Info.DataSize);
- if (LR.Entry->getTotalRefCount() != 0 ||
- LR.Entry->getDeleteThreadId() != std::this_thread::get_id()) {
- // The thread is not in charge of deletion anymore. Give up access to
- // the HDTT map and unset the deletion flag.
- HDTTMap.destroy();
- Info.DelEntry = false;
- }
- }
-
+ bool DelEntry = Info.TPR.Flags.IsOwned;
// 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
@@ -883,7 +854,7 @@
DPxPTR(Itr->second.HstPtrVal), DPxPTR(ShadowHstPtrAddr));
}
// If the struct is to be deallocated, remove the shadow entry.
- if (Info.DelEntry) {
+ if (DelEntry) {
DP("Removing shadow pointer " DPxMOD "\n", DPxPTR((void **)Itr->first));
auto OldItr = Itr;
Itr++;
@@ -898,9 +869,8 @@
// If we are deleting the entry the DataMapMtx is locked and we own the
// entry.
- if (Info.DelEntry) {
- if (!FromMapperBase || FromMapperBase != Info.HstPtrBegin)
- Ret = Device.deallocTgtPtr(HDTTMap, LR, Info.DataSize);
+ if (DelEntry) {
+ Ret = Device.deallocTgtPtr(Info.TPR.Entry, Info.DataSize);
if (Ret != OFFLOAD_SUCCESS) {
REPORT("Deallocating data from device failed.\n");