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 @@ -128,6 +128,23 @@ LookupResult() : Flags({0, 0, 0}), Entry() {} }; +/// This struct will be returned by \p DeviceTy::getOrAllocTgtPtr which provides +/// more data than just a target pointer. +struct TargetPointerResultTy { + struct { + /// If the map table entry is just created + unsigned IsNewEntry : 1; + /// If the pointer is actually a host pointer (when unified memory enabled) + unsigned IsHostPointer : 1; + } Flags = {0, 0}; + + /// The iterator to the corresponding map table entry + HostDataToTargetListTy::iterator MapTableEntry{}; + + /// The corresponding target pointer + void *TargetPointer = nullptr; +}; + /// Map for shadow pointers struct ShadowPtrValTy { void *HstPtrVal; @@ -179,10 +196,12 @@ uint64_t getMapEntryRefCnt(void *HstPtrBegin); LookupResult lookupMapping(void *HstPtrBegin, int64_t Size); - void *getOrAllocTgtPtr(void *HstPtrBegin, void *HstPtrBase, int64_t Size, - map_var_info_t HstPtrName, bool &IsNew, - bool &IsHostPtr, bool IsImplicit, bool UpdateRefCount, - bool HasCloseModifier, bool HasPresentModifier); + TargetPointerResultTy getOrAllocTgtPtr(void *HstPtrBegin, void *HstPtrBase, + int64_t Size, + map_var_info_t HstPtrName, + bool IsImplicit, bool UpdateRefCount, + bool HasCloseModifier, + bool HasPresentModifier); void *getTgtPtrBegin(void *HstPtrBegin, int64_t Size); void *getTgtPtrBegin(void *HstPtrBegin, int64_t Size, bool &IsLast, bool UpdateRefCount, bool &IsHostPtr, 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 @@ -191,50 +191,55 @@ } // Used by targetDataBegin -// Return the target pointer begin (where the data will be moved). +// Return a struct containing target pointer begin (where the data will be +// moved). // Allocate memory if this is the first occurrence of this mapping. // Increment the reference counter. -// If NULL is returned, then either data allocation failed or the user tried -// to do an illegal mapping. -void *DeviceTy::getOrAllocTgtPtr(void *HstPtrBegin, void *HstPtrBase, - int64_t Size, map_var_info_t HstPtrName, - bool &IsNew, bool &IsHostPtr, bool IsImplicit, - bool UpdateRefCount, bool HasCloseModifier, - bool HasPresentModifier) { - void *rc = NULL; - IsHostPtr = false; - IsNew = false; +// If the target pointer is NULL, then either data allocation failed or the user +// tried to do an illegal mapping. +// The returned struct also returns an iterator to the map table entry +// corresponding to the host pointer (if exists), and two flags indicating +// whether the entry is just created, and if the target pointer included is +// actually a host pointer (when unified memory enabled). +TargetPointerResultTy +DeviceTy::getOrAllocTgtPtr(void *HstPtrBegin, void *HstPtrBase, int64_t Size, + map_var_info_t HstPtrName, bool IsImplicit, + bool UpdateRefCount, bool HasCloseModifier, + bool HasPresentModifier) { + void *TargetPointer = NULL; + bool IsNew = false; + bool IsHostPtr = false; DataMapMtx.lock(); - LookupResult lr = lookupMapping(HstPtrBegin, Size); + LookupResult LR = lookupMapping(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 // lead to the IsContained flag to be true - then we must ensure that the // device address is returned even under unified memory conditions. - if (lr.Flags.IsContained || - ((lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter) && IsImplicit)) { - auto &HT = *lr.Entry; - IsNew = false; + if (LR.Flags.IsContained || + ((LR.Flags.ExtendsBefore || LR.Flags.ExtendsAfter) && IsImplicit)) { + auto &HT = *LR.Entry; if (UpdateRefCount) HT.incRefCount(); - uintptr_t tp = HT.TgtPtrBegin + ((uintptr_t)HstPtrBegin - HT.HstPtrBegin); + uintptr_t Ptr = HT.TgtPtrBegin + ((uintptr_t)HstPtrBegin - HT.HstPtrBegin); INFO(OMP_INFOTYPE_MAPPING_EXISTS, DeviceID, "Mapping exists%s with HstPtrBegin=" DPxMOD ", TgtPtrBegin=" DPxMOD ", " "Size=%" PRId64 ", RefCount=%s (%s), Name=%s\n", - (IsImplicit ? " (implicit)" : ""), DPxPTR(HstPtrBegin), DPxPTR(tp), + (IsImplicit ? " (implicit)" : ""), DPxPTR(HstPtrBegin), DPxPTR(Ptr), Size, HT.refCountToStr().c_str(), UpdateRefCount ? "incremented" : "update suppressed", (HstPtrName) ? getNameFromMapping(HstPtrName).c_str() : "unknown"); - rc = (void *)tp; - } else if ((lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter) && !IsImplicit) { + TargetPointer = (void *)Ptr; + } else if ((LR.Flags.ExtendsBefore || LR.Flags.ExtendsAfter) && !IsImplicit) { // Explicit extension of mapped data - not allowed. MESSAGE("explicit extension not allowed: host address specified is " DPxMOD " (%" PRId64 " bytes), but device allocation maps to host at " DPxMOD " (%" PRId64 " bytes)", - DPxPTR(HstPtrBegin), Size, DPxPTR(lr.Entry->HstPtrBegin), - lr.Entry->HstPtrEnd - lr.Entry->HstPtrBegin); + DPxPTR(HstPtrBegin), Size, DPxPTR(Entry->HstPtrBegin), + Entry->HstPtrEnd - Entry->HstPtrBegin); if (HasPresentModifier) MESSAGE("device mapping required by 'present' map type modifier does not " "exist for host address " DPxMOD " (%" PRId64 " bytes)", @@ -252,7 +257,7 @@ "memory\n", DPxPTR((uintptr_t)HstPtrBegin), Size); IsHostPtr = true; - rc = HstPtrBegin; + TargetPointer = HstPtrBegin; } } else if (HasPresentModifier) { DP("Mapping required by 'present' map type modifier does not exist for " @@ -264,24 +269,22 @@ } else if (Size) { // If it is not contained and Size > 0, we should create a new entry for it. IsNew = true; - uintptr_t tp = (uintptr_t)allocData(Size, HstPtrBegin); - const HostDataToTargetTy &newEntry = - *HostDataToTargetMap - .emplace((uintptr_t)HstPtrBase, (uintptr_t)HstPtrBegin, - (uintptr_t)HstPtrBegin + Size, tp, HstPtrName) - .first; + uintptr_t Ptr = (uintptr_t)allocData(Size, HstPtrBegin); + Entry = HostDataToTargetMap + .emplace((uintptr_t)HstPtrBase, (uintptr_t)HstPtrBegin, + (uintptr_t)HstPtrBegin + Size, Ptr, HstPtrName) + .first; INFO(OMP_INFOTYPE_MAPPING_CHANGED, DeviceID, "Creating new map entry with " "HstPtrBegin=" DPxMOD ", TgtPtrBegin=" DPxMOD ", Size=%ld, " "RefCount=%s, Name=%s\n", - DPxPTR(HstPtrBegin), DPxPTR(tp), Size, - newEntry.refCountToStr().c_str(), + DPxPTR(HstPtrBegin), DPxPTR(Ptr), Size, Entry->refCountToStr().c_str(), (HstPtrName) ? getNameFromMapping(HstPtrName).c_str() : "unknown"); - rc = (void *)tp; + TargetPointer = (void *)Ptr; } DataMapMtx.unlock(); - return rc; + return {{IsNew, IsHostPtr}, Entry, TargetPointer}; } // Used by targetDataBegin, targetDataEnd, targetDataUpdate and target. 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 @@ -458,7 +458,7 @@ // Address of pointer on the host and device, respectively. void *Pointer_HstPtrBegin, *PointerTgtPtrBegin; - bool IsNew, Pointer_IsNew; + TargetPointerResultTy Pointer_TPR; bool IsHostPtr = false; bool IsImplicit = arg_types[i] & OMP_TGT_MAPTYPE_IMPLICIT; // Force the creation of a device side copy of the data when: @@ -487,10 +487,11 @@ // entry for a global that might not already be allocated by the time the // PTR_AND_OBJ entry is handled below, and so the allocation might fail // when HasPresentModifier. - PointerTgtPtrBegin = Device.getOrAllocTgtPtr( - HstPtrBase, HstPtrBase, sizeof(void *), nullptr, Pointer_IsNew, - IsHostPtr, IsImplicit, UpdateRef, HasCloseModifier, - HasPresentModifier); + Pointer_TPR = Device.getOrAllocTgtPtr( + HstPtrBase, HstPtrBase, sizeof(void *), nullptr, IsImplicit, + UpdateRef, HasCloseModifier, HasPresentModifier); + PointerTgtPtrBegin = Pointer_TPR.TargetPointer; + IsHostPtr = Pointer_TPR.Flags.IsHostPointer; if (!PointerTgtPtrBegin) { REPORT("Call to getOrAllocTgtPtr returned null pointer (%s).\n", HasPresentModifier ? "'present' map type modifier" @@ -500,7 +501,7 @@ DP("There are %zu bytes allocated at target address " DPxMOD " - is%s new" "\n", sizeof(void *), DPxPTR(PointerTgtPtrBegin), - (Pointer_IsNew ? "" : " not")); + (Pointer_TPR.Flags.IsNewEntry ? "" : " not")); Pointer_HstPtrBegin = HstPtrBase; // modify current entry. HstPtrBase = *(void **)HstPtrBase; @@ -510,9 +511,11 @@ (!FromMapper || i != 0); // subsequently update ref count of pointee } - void *TgtPtrBegin = Device.getOrAllocTgtPtr( - HstPtrBegin, HstPtrBase, data_size, HstPtrName, IsNew, IsHostPtr, - IsImplicit, UpdateRef, HasCloseModifier, HasPresentModifier); + auto TPR = Device.getOrAllocTgtPtr(HstPtrBegin, HstPtrBase, data_size, + HstPtrName, IsImplicit, UpdateRef, + HasCloseModifier, HasPresentModifier); + void *TgtPtrBegin = TPR.TargetPointer; + IsHostPtr = TPR.Flags.IsHostPointer; // If data_size==0, then the argument could be a zero-length pointer to // NULL, so getOrAlloc() returning NULL is not an error. if (!TgtPtrBegin && (data_size || HasPresentModifier)) { @@ -523,7 +526,7 @@ } DP("There are %" PRId64 " bytes allocated at target address " DPxMOD " - is%s new\n", - data_size, DPxPTR(TgtPtrBegin), (IsNew ? "" : " not")); + data_size, DPxPTR(TgtPtrBegin), (TPR.Flags.IsNewEntry ? "" : " not")); if (arg_types[i] & OMP_TGT_MAPTYPE_RETURN_PARAM) { uintptr_t Delta = (uintptr_t)HstPtrBegin - (uintptr_t)HstPtrBase; @@ -536,7 +539,7 @@ bool copy = false; if (!(PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY) || HasCloseModifier) { - if (IsNew || (arg_types[i] & OMP_TGT_MAPTYPE_ALWAYS)) { + if (TPR.Flags.IsNewEntry || (arg_types[i] & OMP_TGT_MAPTYPE_ALWAYS)) { copy = true; } else if ((arg_types[i] & OMP_TGT_MAPTYPE_MEMBER_OF) && !(arg_types[i] & OMP_TGT_MAPTYPE_PTR_AND_OBJ)) {