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
@@ -60,9 +60,7 @@
struct StatesTy {
StatesTy(uint64_t DRC, uint64_t HRC)
- : DynRefCount(DRC), HoldRefCount(HRC),
- MayContainAttachedPointers(false), DeleteThreadId(std::thread::id()) {
- }
+ : DynRefCount(DRC), HoldRefCount(HRC) {}
/// 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
/// OpenACC support.
@@ -101,13 +99,13 @@
/// 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;
+ /// Counter used to coordinate the deletion of an entry. Each target task
+ /// that decides the entry is to be deleted will increment the counter. As
+ /// there is a gap between deciding on deletion and actual deletion, e.g.,
+ /// to wait for the copy back, the task has to decrement the counter before
+ /// deletion is issued. Only if it is 0, and the ref count is 0 too, we
+ /// delete the entry.
+ unsigned short DeleteCount = 0;
};
// When HostDataToTargetTy is used by std::set, std::set::iterator is const
// use unique_ptr to make States mutable.
@@ -148,14 +146,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; }
@@ -229,6 +219,14 @@
return States->MayContainAttachedPointers;
}
+ /// Increment the delete counter indicating that this thread plans to delete
+ /// the entry.
+ void incDeleteCount() const { ++States->DeleteCount; }
+
+ /// Decrement the delete counter prior to deletion. If the return value is 0
+ /// deletion should take place, otherwise the entry is still needed.
+ unsigned decDeleteCount() const { return --States->DeleteCount; }
+
void lock() const { States->UpdateMtx.lock(); }
void unlock() const { States->UpdateMtx.unlock(); }
@@ -298,8 +296,7 @@
TargetPointerResultTy(FlagTy Flags, HostDataToTargetTy *Entry,
void *TargetPointer)
- : Flags(Flags), Entry(Entry), TargetPointer(TargetPointer) {
- }
+ : Flags(Flags), Entry(Entry), TargetPointer(TargetPointer) {}
bool isHostPtr() const { return Flags.IsHostPointer; }
void setIsHostPtr(bool IHP) { Flags.IsHostPointer = IHP; }
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
@@ -383,11 +383,7 @@
if (!UpdateRefCount) {
RefCountAction = " (update suppressed)";
} else if (TPR.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.incDeleteCount();
HT.decRefCount(UseHoldRefCount);
assert(HT.getTotalRefCount() == 0 &&
"Expected zero reference count when deletion is scheduled");
@@ -450,7 +446,6 @@
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.");
DP("Deleting tgt data " DPxMOD " of size %" PRId64 "\n",
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
@@ -622,17 +622,17 @@
/// The mapping type (bitfield).
int64_t ArgType;
- /// 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;
+ /// The entry we currently copy back and which we might delete.
+ HostDataToTargetTy *Entry;
+
PostProcessingInfo(void *HstPtr, int64_t Size, int64_t ArgType, bool DelEntry,
- TargetPointerResultTy TPR)
- : HstPtrBegin(HstPtr), DataSize(Size), ArgType(ArgType), TPR(TPR),
- DelEntry(DelEntry) {}
+ HostDataToTargetTy *Entry)
+ : HstPtrBegin(HstPtr), DataSize(Size), ArgType(ArgType),
+ DelEntry(DelEntry), Entry(Entry) {}
};
/// Apply \p CB to the shadow map pointer entries in the range \p Begin, to
@@ -641,8 +641,7 @@
/// rest of the map is not checked anymore.
template
static void applyToShadowMapEntries(DeviceTy &Device, CBTy CB, void *Begin,
- uintptr_t Size,
- const TargetPointerResultTy &TPR) {
+ uintptr_t Size, HostDataToTargetTy *Entry) {
// If we have an object that is too small to hold a pointer subobject, no need
// to do any checking.
if (Size < sizeof(void *))
@@ -650,7 +649,7 @@
// If the map entry for the object was never marked as containing attached
// pointers, no need to do any checking.
- if (!TPR.getEntry() || !TPR.getEntry()->getMayContainAttachedPointers())
+ if (!Entry || !Entry->getMayContainAttachedPointers())
return;
uintptr_t LB = (uintptr_t)Begin;
@@ -834,8 +833,9 @@
}
// Add pointer to the buffer for post-synchronize processing.
- PostProcessingPtrs.emplace_back(HstPtrBegin, DataSize, ArgTypes[I],
- DelEntry && !TPR.isHostPtr(), TPR);
+ if (auto *Entry = TPR.getEntry())
+ PostProcessingPtrs.emplace_back(HstPtrBegin, DataSize, ArgTypes[I],
+ DelEntry && !TPR.isHostPtr(), Entry);
}
}
@@ -848,7 +848,11 @@
return OFFLOAD_FAIL;
// Deallocate target pointer
+ std::set DeletedEntries;
for (PostProcessingInfo &Info : PostProcessingPtrs) {
+ if (DeletedEntries.count(Info.Entry))
+ continue;
+
// 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
@@ -860,8 +864,9 @@
if (Info.DelEntry) {
LR = Device.lookupMapping(HDTTMap, Info.HstPtrBegin, Info.DataSize);
+ assert(LR.Entry == Info.Entry && "Entry disappeared unexpectedly");
if (LR.Entry->getTotalRefCount() != 0 ||
- LR.Entry->getDeleteThreadId() != std::this_thread::get_id()) {
+ LR.Entry->decDeleteCount() != 0) {
// The thread is not in charge of deletion anymore. Give up access to
// the HDTT map and unset the deletion flag.
HDTTMap.destroy();
@@ -892,18 +897,23 @@
return OFFLOAD_SUCCESS;
};
applyToShadowMapEntries(Device, CB, Info.HstPtrBegin, Info.DataSize,
- Info.TPR);
+ Info.Entry);
// 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 (!Info.DelEntry ||
+ (FromMapperBase && FromMapperBase == Info.HstPtrBegin)) {
+ continue;
+ }
- if (Ret != OFFLOAD_SUCCESS) {
- REPORT("Deallocating data from device failed.\n");
- break;
- }
+ // Remember we deleted the entry in case it shows up in the
+ // PostProcessingPtrs again.
+ DeletedEntries.insert(Info.Entry);
+ auto Ret = Device.deallocTgtPtr(HDTTMap, LR, Info.DataSize);
+
+ if (Ret != OFFLOAD_SUCCESS) {
+ REPORT("Deallocating data from device failed.\n");
+ break;
}
}
@@ -957,7 +967,7 @@
++Itr;
return OFFLOAD_SUCCESS;
};
- applyToShadowMapEntries(Device, CB, HstPtrBegin, ArgSize, TPR);
+ applyToShadowMapEntries(Device, CB, HstPtrBegin, ArgSize, TPR.getEntry());
}
if (ArgType & OMP_TGT_MAPTYPE_TO) {
@@ -980,7 +990,7 @@
++Itr;
return Ret;
};
- applyToShadowMapEntries(Device, CB, HstPtrBegin, ArgSize, TPR);
+ applyToShadowMapEntries(Device, CB, HstPtrBegin, ArgSize, TPR.getEntry());
}
return OFFLOAD_SUCCESS;
}