[Openmp-commits] [openmp] 4e34f06 - [OpenMP][FIX] Ensure exclusive access to the HDTT map
Johannes Doerfert via Openmp-commits
openmp-commits at lists.llvm.org
Fri Mar 25 09:39:54 PDT 2022
Author: Johannes Doerfert
Date: 2022-03-25T11:38:54-05:00
New Revision: 4e34f061d65e7ee45765304088a0a831a203b85b
URL: https://github.com/llvm/llvm-project/commit/4e34f061d65e7ee45765304088a0a831a203b85b
DIFF: https://github.com/llvm/llvm-project/commit/4e34f061d65e7ee45765304088a0a831a203b85b.diff
LOG: [OpenMP][FIX] Ensure exclusive access to the HDTT map
This patch solves two problems with the `HostDataToTargetMap` (HDTT
map) which caused races and crashes before:
1) Any access to the HDTT map needs to be exclusive access. This was not
the case for the "dump table" traversals that could collide with
updates by other threads. The new `Accessor` and `ProtectedObject`
wrappers will ensure we have a hard time introducing similar races in
the future. Note that we could allow multiple concurrent
read-accesses but that feature can be added to the `Accessor` API
later.
2) The elements of the HDTT map were `HostDataToTargetTy` objects which
meant that they could be copied/moved/deleted as the map was changed.
However, we sometimes kept pointers to these elements around after we
gave up the map lock which caused potential races again. The new
indirection through `HostDataToTargetMapKeyTy` will allows us to
modify the map while keeping the (interesting part of the) entries
valid. To offset potential cost we duplicate the ordering key of the
entry which avoids an additional indirect lookup.
We should replace more objects with "protected objects" as we go.
Differential Revision: https://reviews.llvm.org/D121057
Added:
openmp/libomptarget/include/ExclusiveAccess.h
Modified:
openmp/libomptarget/include/device.h
openmp/libomptarget/src/device.cpp
openmp/libomptarget/src/omptarget.cpp
openmp/libomptarget/src/private.h
Removed:
################################################################################
diff --git a/openmp/libomptarget/include/ExclusiveAccess.h b/openmp/libomptarget/include/ExclusiveAccess.h
new file mode 100644
index 0000000000000..ec34e2451efb2
--- /dev/null
+++ b/openmp/libomptarget/include/ExclusiveAccess.h
@@ -0,0 +1,100 @@
+//===---- ExclusiveAccess.h - Helper for exclusive access data structures -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef OMPTARGET_EXCLUSIVE_ACCESS
+#define OMPTARGET_EXCLUSIVE_ACCESS
+
+#include <cstddef>
+#include <cstdint>
+#include <mutex>
+
+/// Forward declaration.
+template <typename Ty> struct Accessor;
+
+/// A protected object is a simple wrapper to allocate an object of type \p Ty
+/// together with a mutex that guards accesses to the object. The only way to
+/// access the object is through the "exclusive accessor" which will lock the
+/// mutex accordingly.
+template <typename Ty> struct ProtectedObj {
+ using AccessorTy = Accessor<Ty>;
+
+ /// Get an exclusive access Accessor object. \p DoNotGetAccess allows to
+ /// create an accessor that is not owning anything based on a boolean
+ /// condition.
+ AccessorTy &&getExclusiveAccessor(bool DoNotGetAccess = false);
+
+private:
+ Ty Obj;
+ std::mutex Mtx;
+ friend struct Accessor<Ty>;
+};
+
+/// Helper to provide transparent exclusive access to protected objects.
+template <typename Ty> struct Accessor {
+ /// Default constructor does not own anything and cannot access anything.
+ Accessor() : Ptr(nullptr) {}
+
+ /// Constructor to get exclusive access by locking the mutex protecting the
+ /// underlying object.
+ Accessor(ProtectedObj<Ty> &PO) : Ptr(&PO) { lock(); }
+
+ /// Constructor to get exclusive access by taking it from \p Other.
+ Accessor(Accessor<Ty> &&Other) : Ptr(Other.Ptr) { Other.Ptr = nullptr; }
+
+ Accessor(Accessor &Other) = delete;
+
+ /// If the object is still owned when the lifetime ends we give up access.
+ ~Accessor() { unlock(); }
+
+ /// Give up access to the underlying object, virtually "destroying" the
+ /// accessor even if the object is still life.
+ void destroy() {
+ unlock();
+ Ptr = nullptr;
+ }
+
+ /// Provide transparent access to the underlying object.
+ Ty &operator*() {
+ assert(Ptr && "Trying to access an object through a non-owning (or "
+ "destroyed) accessor!");
+ return Ptr->Obj;
+ }
+ Ty *operator->() {
+ assert(Ptr && "Trying to access an object through a non-owning (or "
+ "destroyed) accessor!");
+ return &Ptr->Obj;
+ }
+
+private:
+ /// Lock the underlying object if there is one.
+ void lock() {
+ if (Ptr)
+ Ptr->Mtx.lock();
+ }
+
+ /// Unlock the underlying object if there is one.
+ void unlock() {
+ if (Ptr)
+ Ptr->Mtx.unlock();
+ }
+
+ /// Pointer to the underlying object or null if the accessor lost access,
+ /// e.g., after a destroy call.
+ ProtectedObj<Ty> *Ptr;
+};
+
+template <typename Ty>
+Accessor<Ty> &&ProtectedObj<Ty>::getExclusiveAccessor(bool DoNotGetAccess) {
+ if (DoNotGetAccess)
+ return std::move(Accessor<Ty>());
+ return std::move(Accessor<Ty>(*this));
+}
+
+#endif
diff --git a/openmp/libomptarget/include/device.h b/openmp/libomptarget/include/device.h
index 17ab2ed4e2ce5..cc5170e4c5722 100644
--- a/openmp/libomptarget/include/device.h
+++ b/openmp/libomptarget/include/device.h
@@ -15,6 +15,7 @@
#include <cassert>
#include <cstddef>
+#include <cstdint>
#include <list>
#include <map>
#include <memory>
@@ -22,6 +23,7 @@
#include <set>
#include <vector>
+#include "ExclusiveAccess.h"
#include "omptarget.h"
#include "rtl.h"
@@ -214,20 +216,32 @@ struct HostDataToTargetTy {
void unlock() const { States->UpdateMtx.unlock(); }
};
-typedef uintptr_t HstPtrBeginTy;
-inline bool operator<(const HostDataToTargetTy &lhs, const HstPtrBeginTy &rhs) {
- return lhs.HstPtrBegin < rhs;
+/// Wrapper around the HostDataToTargetTy to be used in the HDTT map. In
+/// addition to the HDTT pointer we store the key value explicitly. This
+/// allows the set to inspect (sort/search/...) this entry without an additional
+/// load of HDTT. HDTT is a pointer to allow the modification of the set without
+/// invalidating HDTT entries which can now be inspected at the same time.
+struct HostDataToTargetMapKeyTy {
+ uintptr_t KeyValue;
+
+ HostDataToTargetMapKeyTy(void *Key) : KeyValue(uintptr_t(Key)) {}
+ HostDataToTargetMapKeyTy(HostDataToTargetTy *HDTT)
+ : KeyValue(HDTT->HstPtrBegin), HDTT(HDTT) {}
+ HostDataToTargetTy *HDTT;
+};
+inline bool operator<(const HostDataToTargetMapKeyTy &lhs,
+ const uintptr_t &rhs) {
+ return lhs.KeyValue < rhs;
}
-inline bool operator<(const HstPtrBeginTy &lhs, const HostDataToTargetTy &rhs) {
- return lhs < rhs.HstPtrBegin;
+inline bool operator<(const uintptr_t &lhs,
+ const HostDataToTargetMapKeyTy &rhs) {
+ return lhs < rhs.KeyValue;
}
-inline bool operator<(const HostDataToTargetTy &lhs,
- const HostDataToTargetTy &rhs) {
- return lhs.HstPtrBegin < rhs.HstPtrBegin;
+inline bool operator<(const HostDataToTargetMapKeyTy &lhs,
+ const HostDataToTargetMapKeyTy &rhs) {
+ return lhs.KeyValue < rhs.KeyValue;
}
-typedef std::set<HostDataToTargetTy, std::less<>> HostDataToTargetListTy;
-
struct LookupResult {
struct {
unsigned IsContained : 1;
@@ -235,7 +249,8 @@ struct LookupResult {
unsigned ExtendsAfter : 1;
} Flags;
- HostDataToTargetListTy::iterator Entry;
+ /// The corresponding map table entry which is stable.
+ HostDataToTargetTy *Entry = nullptr;
LookupResult() : Flags({0, 0, 0}), Entry() {}
};
@@ -250,8 +265,8 @@ struct TargetPointerResultTy {
unsigned IsHostPointer : 1;
} Flags = {0, 0};
- /// The iterator to the corresponding map table entry
- HostDataToTargetListTy::iterator MapTableEntry{};
+ /// The corresponding map table entry which is stable.
+ HostDataToTargetTy *Entry = nullptr;
/// The corresponding target pointer
void *TargetPointer = nullptr;
@@ -282,12 +297,24 @@ struct DeviceTy {
std::once_flag InitFlag;
bool HasPendingGlobals;
- HostDataToTargetListTy HostDataToTargetMap;
+ /// Host data to device map type with a wrapper key indirection that allows
+ /// concurrent modification of the entries without invalidating the underlying
+ /// entries.
+ using HostDataToTargetListTy =
+ std::set<HostDataToTargetMapKeyTy, std::less<>>;
+
+ /// The HDTTMap is a protected object that can only be accessed by one thread
+ /// at a time.
+ ProtectedObj<HostDataToTargetListTy> HostDataToTargetMap;
+
+ /// The type used to access the HDTT map.
+ using HDTTMapAccessorTy = decltype(HostDataToTargetMap)::AccessorTy;
+
PendingCtorsDtorsPerLibrary PendingCtorsDtors;
ShadowPtrListTy ShadowPtrMap;
- std::mutex DataMapMtx, PendingGlobalsMtx, ShadowMtx;
+ std::mutex PendingGlobalsMtx, ShadowMtx;
// NOTE: Once libomp gains full target-task support, this state should be
// moved into the target task in libomp.
@@ -303,7 +330,11 @@ struct DeviceTy {
// Return true if data can be copied to DstDevice directly
bool isDataExchangable(const DeviceTy &DstDevice);
- LookupResult lookupMapping(void *HstPtrBegin, int64_t Size);
+ /// Lookup the mapping of \p HstPtrBegin in \p HDTTMap. The accessor ensures
+ /// exclusive access to the HDTT map.
+ LookupResult lookupMapping(HDTTMapAccessorTy &HDTTMap, void *HstPtrBegin,
+ int64_t Size);
+
/// Get the target pointer based on host pointer begin and base. If the
/// mapping already exists, the target pointer will be returned directly. In
/// addition, if required, the memory region pointed by \p HstPtrBegin of size
@@ -320,7 +351,12 @@ struct DeviceTy {
bool HasFlagAlways, bool IsImplicit, bool UpdateRefCount,
bool HasCloseModifier, bool HasPresentModifier,
bool HasHoldModifier, AsyncInfoTy &AsyncInfo);
- void *getTgtPtrBegin(void *HstPtrBegin, int64_t Size);
+
+ /// Return the target pointer for \p HstPtrBegin in \p HDTTMap. The accessor
+ /// ensures exclusive access to the HDTT map.
+ void *getTgtPtrBegin(HDTTMapAccessorTy &HDTTMap, void *HstPtrBegin,
+ int64_t Size);
+
TargetPointerResultTy getTgtPtrBegin(void *HstPtrBegin, int64_t Size,
bool &IsLast, bool UpdateRefCount,
bool UseHoldRefCount, bool &IsHostPtr,
diff --git a/openmp/libomptarget/src/device.cpp b/openmp/libomptarget/src/device.cpp
index bcad793e73e9e..df9e422c7e245 100644
--- a/openmp/libomptarget/src/device.cpp
+++ b/openmp/libomptarget/src/device.cpp
@@ -17,6 +17,7 @@
#include <cassert>
#include <climits>
+#include <cstdint>
#include <cstdio>
#include <string>
@@ -49,8 +50,8 @@ int HostDataToTargetTy::addEventIfNecessary(DeviceTy &Device,
DeviceTy::DeviceTy(RTLInfoTy *RTL)
: DeviceID(-1), RTL(RTL), RTLDeviceID(-1), IsInit(false), InitFlag(),
- HasPendingGlobals(false), HostDataToTargetMap(), PendingCtorsDtors(),
- ShadowPtrMap(), DataMapMtx(), PendingGlobalsMtx(), ShadowMtx() {}
+ HasPendingGlobals(false), PendingCtorsDtors(), ShadowPtrMap(),
+ PendingGlobalsMtx(), ShadowMtx() {}
DeviceTy::~DeviceTy() {
if (DeviceID == -1 || !(getInfoLevel() & OMP_INFOTYPE_DUMP_TABLE))
@@ -61,14 +62,15 @@ DeviceTy::~DeviceTy() {
}
int DeviceTy::associatePtr(void *HstPtrBegin, void *TgtPtrBegin, int64_t Size) {
- std::lock_guard<decltype(DataMapMtx)> LG(DataMapMtx);
+ HDTTMapAccessorTy HDTTMap = HostDataToTargetMap.getExclusiveAccessor();
// Check if entry exists
- auto search = HostDataToTargetMap.find(HstPtrBeginTy{(uintptr_t)HstPtrBegin});
- if (search != HostDataToTargetMap.end()) {
+ auto It = HDTTMap->find(HstPtrBegin);
+ if (It != HDTTMap->end()) {
+ HostDataToTargetTy &HDTT = *It->HDTT;
// Mapping already exists
- bool isValid = search->HstPtrEnd == (uintptr_t)HstPtrBegin + Size &&
- search->TgtPtrBegin == (uintptr_t)TgtPtrBegin;
+ bool isValid = HDTT.HstPtrEnd == (uintptr_t)HstPtrBegin + Size &&
+ HDTT.TgtPtrBegin == (uintptr_t)TgtPtrBegin;
if (isValid) {
DP("Attempt to re-associate the same device ptr+offset with the same "
"host ptr, nothing to do\n");
@@ -82,15 +84,15 @@ int DeviceTy::associatePtr(void *HstPtrBegin, void *TgtPtrBegin, int64_t Size) {
// Mapping does not exist, allocate it with refCount=INF
const HostDataToTargetTy &newEntry =
- *HostDataToTargetMap
- .emplace(
+ *HDTTMap
+ ->emplace(new HostDataToTargetTy(
/*HstPtrBase=*/(uintptr_t)HstPtrBegin,
/*HstPtrBegin=*/(uintptr_t)HstPtrBegin,
/*HstPtrEnd=*/(uintptr_t)HstPtrBegin + Size,
/*TgtPtrBegin=*/(uintptr_t)TgtPtrBegin,
/*UseHoldRefCount=*/false, /*Name=*/nullptr,
- /*IsRefCountINF=*/true)
- .first;
+ /*IsRefCountINF=*/true))
+ .first->HDTT;
DP("Creating new map entry: HstBase=" DPxMOD ", HstBegin=" DPxMOD
", HstEnd=" DPxMOD ", TgtBegin=" DPxMOD ", DynRefCount=%s, "
"HoldRefCount=%s\n",
@@ -103,23 +105,25 @@ int DeviceTy::associatePtr(void *HstPtrBegin, void *TgtPtrBegin, int64_t Size) {
}
int DeviceTy::disassociatePtr(void *HstPtrBegin) {
- std::lock_guard<decltype(DataMapMtx)> LG(DataMapMtx);
+ HDTTMapAccessorTy HDTTMap = HostDataToTargetMap.getExclusiveAccessor();
- auto search = HostDataToTargetMap.find(HstPtrBeginTy{(uintptr_t)HstPtrBegin});
- if (search != HostDataToTargetMap.end()) {
+ auto It = HDTTMap->find(HstPtrBegin);
+ if (It != HDTTMap->end()) {
+ HostDataToTargetTy &HDTT = *It->HDTT;
// Mapping exists
- if (search->getHoldRefCount()) {
+ if (HDTT.getHoldRefCount()) {
// This is based on OpenACC 3.1, sec 3.2.33 "acc_unmap_data", L3656-3657:
// "It is an error to call acc_unmap_data if the structured reference
// count for the pointer is not zero."
REPORT("Trying to disassociate a pointer with a non-zero hold reference "
"count\n");
- } else if (search->isDynRefCountInf()) {
+ } else if (HDTT.isDynRefCountInf()) {
DP("Association found, removing it\n");
- void *Event = search->getEvent();
+ void *Event = HDTT.getEvent();
+ delete &HDTT;
if (Event)
destroyEvent(Event);
- HostDataToTargetMap.erase(search);
+ HDTTMap->erase(It);
return OFFLOAD_SUCCESS;
} else {
REPORT("Trying to disassociate a pointer which was not mapped via "
@@ -133,20 +137,22 @@ int DeviceTy::disassociatePtr(void *HstPtrBegin) {
return OFFLOAD_FAIL;
}
-LookupResult DeviceTy::lookupMapping(void *HstPtrBegin, int64_t Size) {
+LookupResult DeviceTy::lookupMapping(HDTTMapAccessorTy &HDTTMap,
+ void *HstPtrBegin, int64_t Size) {
+
uintptr_t hp = (uintptr_t)HstPtrBegin;
LookupResult lr;
DP("Looking up mapping(HstPtrBegin=" DPxMOD ", Size=%" PRId64 ")...\n",
DPxPTR(hp), Size);
- if (HostDataToTargetMap.empty())
+ if (HDTTMap->empty())
return lr;
- auto upper = HostDataToTargetMap.upper_bound(hp);
+ auto upper = HDTTMap->upper_bound(hp);
// check the left bin
- if (upper != HostDataToTargetMap.begin()) {
- lr.Entry = std::prev(upper);
+ if (upper != HDTTMap->begin()) {
+ lr.Entry = std::prev(upper)->HDTT;
auto &HT = *lr.Entry;
// Is it contained?
lr.Flags.IsContained = hp >= HT.HstPtrBegin && hp < HT.HstPtrEnd &&
@@ -157,8 +163,8 @@ LookupResult DeviceTy::lookupMapping(void *HstPtrBegin, int64_t Size) {
// check the right bin
if (!(lr.Flags.IsContained || lr.Flags.ExtendsAfter) &&
- upper != HostDataToTargetMap.end()) {
- lr.Entry = upper;
+ upper != HDTTMap->end()) {
+ lr.Entry = upper->HDTT;
auto &HT = *lr.Entry;
// Does it extend into an already mapped region?
lr.Flags.ExtendsBefore =
@@ -184,14 +190,14 @@ TargetPointerResultTy DeviceTy::getTargetPointer(
map_var_info_t HstPtrName, bool HasFlagTo, bool HasFlagAlways,
bool IsImplicit, bool UpdateRefCount, bool HasCloseModifier,
bool HasPresentModifier, bool HasHoldModifier, AsyncInfoTy &AsyncInfo) {
+ HDTTMapAccessorTy HDTTMap = HostDataToTargetMap.getExclusiveAccessor();
+
void *TargetPointer = nullptr;
bool IsHostPtr = false;
bool IsNew = false;
- DataMapMtx.lock();
-
- LookupResult LR = lookupMapping(HstPtrBegin, Size);
- auto Entry = LR.Entry;
+ LookupResult LR = lookupMapping(HDTTMap, HstPtrBegin, Size);
+ auto *Entry = LR.Entry;
// Check if the pointer is contained.
// If a variable is mapped to the device manually by the user - which would
@@ -260,11 +266,12 @@ TargetPointerResultTy DeviceTy::getTargetPointer(
// If it is not contained and Size > 0, we should create a new entry for it.
IsNew = true;
uintptr_t Ptr = (uintptr_t)allocData(Size, HstPtrBegin);
- Entry = HostDataToTargetMap
- .emplace((uintptr_t)HstPtrBase, (uintptr_t)HstPtrBegin,
- (uintptr_t)HstPtrBegin + Size, Ptr, HasHoldModifier,
- HstPtrName)
- .first;
+ Entry = HDTTMap
+ ->emplace(new HostDataToTargetTy(
+ (uintptr_t)HstPtrBase, (uintptr_t)HstPtrBegin,
+ (uintptr_t)HstPtrBegin + Size, Ptr, HasHoldModifier,
+ HstPtrName))
+ .first->HDTT;
INFO(OMP_INFOTYPE_MAPPING_CHANGED, DeviceID,
"Creating new map entry with "
"HstPtrBegin=" DPxMOD ", TgtPtrBegin=" DPxMOD ", Size=%ld, "
@@ -282,7 +289,7 @@ TargetPointerResultTy DeviceTy::getTargetPointer(
// thread that could issue data movement will get the right result.
std::lock_guard<decltype(*Entry)> LG(*Entry);
// Release the mapping table lock right after the entry is locked.
- DataMapMtx.unlock();
+ HDTTMap.destroy();
DP("Moving %" PRId64 " bytes (hst:" DPxMOD ") -> (tgt:" DPxMOD ")\n", Size,
DPxPTR(HstPtrBegin), DPxPTR(TargetPointer));
@@ -296,16 +303,15 @@ TargetPointerResultTy DeviceTy::getTargetPointer(
TargetPointer = nullptr;
} else if (Entry->addEventIfNecessary(*this, AsyncInfo) != OFFLOAD_SUCCESS)
return {{false /* IsNewEntry */, false /* IsHostPointer */},
- {} /* MapTableEntry */,
+ nullptr /* Entry */,
nullptr /* TargetPointer */};
} else {
// Release the mapping table lock directly.
- DataMapMtx.unlock();
+ HDTTMap.destroy();
// If not a host pointer and no present modifier, we need to wait for the
// event if it exists.
// Note: Entry might be nullptr because of zero length array section.
- if (Entry != HostDataToTargetListTy::iterator() && !IsHostPtr &&
- !HasPresentModifier) {
+ if (Entry && !IsHostPtr && !HasPresentModifier) {
std::lock_guard<decltype(*Entry)> LG(*Entry);
void *Event = Entry->getEvent();
if (Event) {
@@ -315,7 +321,7 @@ TargetPointerResultTy DeviceTy::getTargetPointer(
// case of any data race.
REPORT("Failed to wait for event " DPxMOD ".\n", DPxPTR(Event));
return {{false /* IsNewEntry */, false /* IsHostPointer */},
- {} /* MapTableEntry */,
+ nullptr /* Entry */,
nullptr /* TargetPointer */};
}
}
@@ -332,12 +338,13 @@ TargetPointerResultTy
DeviceTy::getTgtPtrBegin(void *HstPtrBegin, int64_t Size, bool &IsLast,
bool UpdateRefCount, bool UseHoldRefCount,
bool &IsHostPtr, bool MustContain, bool ForceDelete) {
+ HDTTMapAccessorTy HDTTMap = HostDataToTargetMap.getExclusiveAccessor();
+
void *TargetPointer = NULL;
bool IsNew = false;
IsHostPtr = false;
IsLast = false;
- std::lock_guard<decltype(DataMapMtx)> LG(DataMapMtx);
- LookupResult lr = lookupMapping(HstPtrBegin, Size);
+ LookupResult lr = lookupMapping(HDTTMap, HstPtrBegin, Size);
if (lr.Flags.IsContained ||
(!MustContain && (lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter))) {
@@ -391,10 +398,10 @@ DeviceTy::getTgtPtrBegin(void *HstPtrBegin, int64_t Size, bool &IsLast,
}
// Return the target pointer begin (where the data will be moved).
-// Lock-free version called when loading global symbols from the fat binary.
-void *DeviceTy::getTgtPtrBegin(void *HstPtrBegin, int64_t Size) {
+void *DeviceTy::getTgtPtrBegin(HDTTMapAccessorTy &HDTTMap, void *HstPtrBegin,
+ int64_t Size) {
uintptr_t hp = (uintptr_t)HstPtrBegin;
- LookupResult lr = lookupMapping(HstPtrBegin, Size);
+ LookupResult lr = lookupMapping(HDTTMap, HstPtrBegin, Size);
if (lr.Flags.IsContained || lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter) {
auto &HT = *lr.Entry;
uintptr_t tp = HT.TgtPtrBegin + (hp - HT.HstPtrBegin);
@@ -406,11 +413,11 @@ void *DeviceTy::getTgtPtrBegin(void *HstPtrBegin, int64_t Size) {
int DeviceTy::deallocTgtPtr(void *HstPtrBegin, int64_t Size,
bool HasHoldModifier) {
- std::lock_guard<decltype(DataMapMtx)> LG(DataMapMtx);
+ HDTTMapAccessorTy HDTTMap = HostDataToTargetMap.getExclusiveAccessor();
// Check if the pointer is contained in any sub-nodes.
int Ret = OFFLOAD_SUCCESS;
- LookupResult lr = lookupMapping(HstPtrBegin, Size);
+ LookupResult lr = lookupMapping(HDTTMap, HstPtrBegin, Size);
if (lr.Flags.IsContained || lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter) {
auto &HT = *lr.Entry;
if (HT.decRefCount(HasHoldModifier) == 0) {
@@ -424,7 +431,8 @@ int DeviceTy::deallocTgtPtr(void *HstPtrBegin, int64_t Size,
(HT.HstPtrName) ? getNameFromMapping(HT.HstPtrName).c_str()
: "unknown");
void *Event = lr.Entry->getEvent();
- HostDataToTargetMap.erase(lr.Entry);
+ HDTTMap->erase(lr.Entry);
+ delete lr.Entry;
if (Event && destroyEvent(Event) != OFFLOAD_SUCCESS) {
REPORT("Failed to destroy event " DPxMOD "\n", DPxPTR(Event));
Ret = OFFLOAD_FAIL;
@@ -492,7 +500,8 @@ int32_t DeviceTy::deleteData(void *TgtPtrBegin) {
int32_t DeviceTy::submitData(void *TgtPtrBegin, void *HstPtrBegin, int64_t Size,
AsyncInfoTy &AsyncInfo) {
if (getInfoLevel() & OMP_INFOTYPE_DATA_TRANSFER) {
- LookupResult LR = lookupMapping(HstPtrBegin, Size);
+ HDTTMapAccessorTy HDTTMap = HostDataToTargetMap.getExclusiveAccessor();
+ LookupResult LR = lookupMapping(HDTTMap, HstPtrBegin, Size);
auto *HT = &*LR.Entry;
INFO(OMP_INFOTYPE_DATA_TRANSFER, DeviceID,
@@ -514,7 +523,8 @@ int32_t DeviceTy::submitData(void *TgtPtrBegin, void *HstPtrBegin, int64_t Size,
int32_t DeviceTy::retrieveData(void *HstPtrBegin, void *TgtPtrBegin,
int64_t Size, AsyncInfoTy &AsyncInfo) {
if (getInfoLevel() & OMP_INFOTYPE_DATA_TRANSFER) {
- LookupResult LR = lookupMapping(HstPtrBegin, Size);
+ HDTTMapAccessorTy HDTTMap = HostDataToTargetMap.getExclusiveAccessor();
+ LookupResult LR = lookupMapping(HDTTMap, HstPtrBegin, Size);
auto *HT = &*LR.Entry;
INFO(OMP_INFOTYPE_DATA_TRANSFER, DeviceID,
"Copying data from device to host, TgtPtr=" DPxMOD ", HstPtr=" DPxMOD
diff --git a/openmp/libomptarget/src/omptarget.cpp b/openmp/libomptarget/src/omptarget.cpp
index de9f00ae617d8..d230bcdf2c8d8 100644
--- a/openmp/libomptarget/src/omptarget.cpp
+++ b/openmp/libomptarget/src/omptarget.cpp
@@ -135,8 +135,8 @@ static int InitLibrary(DeviceTy &Device) {
break;
}
- // process global data that needs to be mapped.
- std::lock_guard<decltype(Device.DataMapMtx)> LG(Device.DataMapMtx);
+ DeviceTy::HDTTMapAccessorTy HDTTMap =
+ Device.HostDataToTargetMap.getExclusiveAccessor();
__tgt_target_table *HostTable = &TransTable->HostTable;
for (__tgt_offload_entry *CurrDeviceEntry = TargetTable->EntriesBegin,
@@ -153,21 +153,23 @@ static int InitLibrary(DeviceTy &Device) {
// 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.
- if (Device.getTgtPtrBegin(CurrHostEntry->addr, CurrHostEntry->size))
+ if (Device.getTgtPtrBegin(HDTTMap, CurrHostEntry->addr,
+ CurrHostEntry->size))
continue;
+
DP("Add mapping from host " DPxMOD " to device " DPxMOD
" with size %zu"
"\n",
DPxPTR(CurrHostEntry->addr), DPxPTR(CurrDeviceEntry->addr),
CurrDeviceEntry->size);
- Device.HostDataToTargetMap.emplace(
+ HDTTMap->emplace(new HostDataToTargetTy(
(uintptr_t)CurrHostEntry->addr /*HstPtrBase*/,
(uintptr_t)CurrHostEntry->addr /*HstPtrBegin*/,
(uintptr_t)CurrHostEntry->addr +
CurrHostEntry->size /*HstPtrEnd*/,
(uintptr_t)CurrDeviceEntry->addr /*TgtPtrBegin*/,
false /*UseHoldRefCount*/, nullptr /*Name*/,
- true /*IsRefCountINF*/);
+ true /*IsRefCountINF*/));
}
}
}
@@ -572,13 +574,12 @@ int targetDataBegin(ident_t *loc, DeviceTy &Device, int32_t arg_num,
// create or update shadow pointers for this entry
Device.ShadowPtrMap[Pointer_HstPtrBegin] = {
HstPtrBase, PointerTgtPtrBegin, ExpectedTgtPtrBase};
- Pointer_TPR.MapTableEntry->setMayContainAttachedPointers();
+ Pointer_TPR.Entry->setMayContainAttachedPointers();
UpdateDevPtr = true;
}
if (UpdateDevPtr) {
- std::lock_guard<decltype(*Pointer_TPR.MapTableEntry)> LG(
- *Pointer_TPR.MapTableEntry);
+ std::lock_guard<decltype(*Pointer_TPR.Entry)> LG(*Pointer_TPR.Entry);
Device.ShadowMtx.unlock();
DP("Update pointer (" DPxMOD ") -> [" DPxMOD "]\n",
@@ -593,7 +594,7 @@ int targetDataBegin(ident_t *loc, DeviceTy &Device, int32_t arg_num,
REPORT("Copying data to device failed.\n");
return OFFLOAD_FAIL;
}
- if (Pointer_TPR.MapTableEntry->addEventIfNecessary(Device, AsyncInfo) !=
+ if (Pointer_TPR.Entry->addEventIfNecessary(Device, AsyncInfo) !=
OFFLOAD_SUCCESS)
return OFFLOAD_FAIL;
} else
@@ -634,8 +635,7 @@ static void applyToShadowMapEntries(DeviceTy &Device, CBTy CB, void *Begin,
// If the map entry for the object was never marked as containing attached
// pointers, no need to do any checking.
- if (TPR.MapTableEntry == HostDataToTargetListTy::iterator{} ||
- !TPR.MapTableEntry->getMayContainAttachedPointers())
+ if (!TPR.Entry || !TPR.Entry->getMayContainAttachedPointers())
return;
uintptr_t LB = (uintptr_t)Begin;
diff --git a/openmp/libomptarget/src/private.h b/openmp/libomptarget/src/private.h
index c2fc4c4f81180..da77b41308ee7 100644
--- a/openmp/libomptarget/src/private.h
+++ b/openmp/libomptarget/src/private.h
@@ -125,7 +125,9 @@ void __kmpc_omp_wait_deps(ident_t *loc_ref, kmp_int32 gtid, kmp_int32 ndeps,
/// dump a table of all the host-target pointer pairs on failure
static inline void dumpTargetPointerMappings(const ident_t *Loc,
DeviceTy &Device) {
- if (Device.HostDataToTargetMap.empty())
+ DeviceTy::HDTTMapAccessorTy HDTTMap =
+ Device.HostDataToTargetMap.getExclusiveAccessor();
+ if (HDTTMap->empty())
return;
SourceInfo Kernel(Loc);
@@ -135,18 +137,16 @@ static inline void dumpTargetPointerMappings(const ident_t *Loc,
INFO(OMP_INFOTYPE_ALL, Device.DeviceID, "%-18s %-18s %s %s %s %s\n",
"Host Ptr", "Target Ptr", "Size (B)", "DynRefCount", "HoldRefCount",
"Declaration");
- Device.DataMapMtx.lock();
- for (const auto &HostTargetMap : Device.HostDataToTargetMap) {
- SourceInfo Info(HostTargetMap.HstPtrName);
+ for (const auto &It : *HDTTMap) {
+ HostDataToTargetTy &HDTT = *It.HDTT;
+ SourceInfo Info(HDTT.HstPtrName);
INFO(OMP_INFOTYPE_ALL, Device.DeviceID,
DPxMOD " " DPxMOD " %-8" PRIuPTR " %-11s %-12s %s at %s:%d:%d\n",
- DPxPTR(HostTargetMap.HstPtrBegin), DPxPTR(HostTargetMap.TgtPtrBegin),
- HostTargetMap.HstPtrEnd - HostTargetMap.HstPtrBegin,
- HostTargetMap.dynRefCountToStr().c_str(),
- HostTargetMap.holdRefCountToStr().c_str(), Info.getName(),
- Info.getFilename(), Info.getLine(), Info.getColumn());
+ DPxPTR(HDTT.HstPtrBegin), DPxPTR(HDTT.TgtPtrBegin),
+ HDTT.HstPtrEnd - HDTT.HstPtrBegin, HDTT.dynRefCountToStr().c_str(),
+ HDTT.holdRefCountToStr().c_str(), Info.getName(), Info.getFilename(),
+ Info.getLine(), Info.getColumn());
}
- Device.DataMapMtx.unlock();
}
////////////////////////////////////////////////////////////////////////////////
More information about the Openmp-commits
mailing list