Index: openmp/libomptarget/src/device.h =================================================================== --- openmp/libomptarget/src/device.h +++ openmp/libomptarget/src/device.h @@ -88,6 +88,10 @@ } bool isRefCountInf() const { return RefCount == INFRefCount; } + + std::string refCountToStr() const { + return isRefCountInf() ? "INF" : std::to_string(getRefCount()); + } }; typedef uintptr_t HstPtrBeginTy; Index: openmp/libomptarget/src/device.cpp =================================================================== --- openmp/libomptarget/src/device.cpp +++ openmp/libomptarget/src/device.cpp @@ -79,13 +79,14 @@ HostDataToTargetTy newEntry((uintptr_t)HstPtrBegin /*HstPtrBase*/, (uintptr_t)HstPtrBegin /*HstPtrBegin*/, (uintptr_t)HstPtrBegin + Size /*HstPtrEnd*/, - (uintptr_t)TgtPtrBegin /*TgtPtrBegin*/, nullptr, - true /*IsRefCountINF*/); + (uintptr_t)TgtPtrBegin /*TgtPtrBegin*/, + nullptr /*Name*/, true /*IsRefCountINF*/); DP("Creating new map entry: HstBase=" DPxMOD ", HstBegin=" DPxMOD - ", HstEnd=" DPxMOD ", TgtBegin=" DPxMOD "\n", + ", HstEnd=" DPxMOD ", TgtBegin=" DPxMOD ", RefCount=%s\n", DPxPTR(newEntry.HstPtrBase), DPxPTR(newEntry.HstPtrBegin), - DPxPTR(newEntry.HstPtrEnd), DPxPTR(newEntry.TgtPtrBegin)); + DPxPTR(newEntry.HstPtrEnd), DPxPTR(newEntry.TgtPtrBegin), + newEntry.refCountToStr().c_str()); HostDataToTargetMap.insert(newEntry); DataMapMtx.unlock(); @@ -212,18 +213,20 @@ ((lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter) && IsImplicit)) { auto &HT = *lr.Entry; IsNew = false; - - if (UpdateRefCount) + const char *RefCountAction; + if (!UpdateRefCount) + RefCountAction = "update suppressed"; + else { + RefCountAction = "incremented"; HT.incRefCount(); - + } uintptr_t tp = HT.TgtPtrBegin + ((uintptr_t)HstPtrBegin - HT.HstPtrBegin); INFO(OMP_INFOTYPE_MAPPING_EXISTS, DeviceID, "Mapping exists%s with HstPtrBegin=" DPxMOD ", TgtPtrBegin=" DPxMOD ", " - "Size=%" PRId64 ",%s RefCount=%s, Name=%s\n", + "Size=%" PRId64 ", RefCount=%s (%s), Name=%s\n", (IsImplicit ? " (implicit)" : ""), DPxPTR(HstPtrBegin), DPxPTR(tp), - Size, (UpdateRefCount ? " updated" : ""), - HT.isRefCountInf() ? "INF" : std::to_string(HT.getRefCount()).c_str(), + Size, HT.refCountToStr().c_str(), RefCountAction, (HstPtrName) ? getNameFromMapping(HstPtrName).c_str() : "unknown"); rc = (void *)tp; } else if ((lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter) && !IsImplicit) { @@ -247,9 +250,9 @@ // In addition to the mapping rules above, the close map modifier forces the // mapping of the variable to the device. if (Size) { - DP("Return HstPtrBegin " DPxMOD " Size=%" PRId64 " RefCount=%s\n", - DPxPTR((uintptr_t)HstPtrBegin), Size, - (UpdateRefCount ? " updated" : "")); + DP("Return HstPtrBegin " DPxMOD " Size=%" PRId64 " for unified shared " + "memory\n", + DPxPTR((uintptr_t)HstPtrBegin), Size); IsHostPtr = true; rc = HstPtrBegin; } @@ -264,14 +267,16 @@ // 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); + HostDataToTargetTy newEntry((uintptr_t)HstPtrBase, (uintptr_t)HstPtrBegin, + (uintptr_t)HstPtrBegin + Size, tp, HstPtrName); INFO(OMP_INFOTYPE_MAPPING_CHANGED, DeviceID, "Creating new map entry with " - "HstPtrBegin=" DPxMOD ", TgtPtrBegin=" DPxMOD ", Size=%ld, Name=%s\n", + "HstPtrBegin=" DPxMOD ", TgtPtrBegin=" DPxMOD ", Size=%ld, " + "RefCount=%s, Name=%s\n", DPxPTR(HstPtrBegin), DPxPTR(tp), Size, + newEntry.refCountToStr().c_str(), (HstPtrName) ? getNameFromMapping(HstPtrName).c_str() : "unknown"); - HostDataToTargetMap.emplace( - HostDataToTargetTy((uintptr_t)HstPtrBase, (uintptr_t)HstPtrBegin, - (uintptr_t)HstPtrBegin + Size, tp, HstPtrName)); + HostDataToTargetMap.insert(newEntry); rc = (void *)tp; } @@ -294,25 +299,35 @@ if (lr.Flags.IsContained || (!MustContain && (lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter))) { auto &HT = *lr.Entry; + // We do not decrement the reference count to zero here. deallocTgtPtr does + // that atomically with removing the mapping. Otherwise, before this thread + // removed the mapping in deallocTgtPtr, another thread could retrieve the + // mapping, increment and decrement back to zero, and then both threads + // would try to remove the mapping, resulting in a double free. IsLast = HT.getRefCount() == 1; - - if (!IsLast && UpdateRefCount) + const char *RefCountAction; + if (!UpdateRefCount) + RefCountAction = "update suppressed"; + else if (IsLast) + RefCountAction = "deferred final decrement"; + else { + RefCountAction = "decremented"; HT.decRefCount(); - + } uintptr_t tp = HT.TgtPtrBegin + ((uintptr_t)HstPtrBegin - HT.HstPtrBegin); - DP("Mapping exists with HstPtrBegin=" DPxMOD ", TgtPtrBegin=" DPxMOD ", " - "Size=%" PRId64 ",%s RefCount=%s\n", - DPxPTR(HstPtrBegin), DPxPTR(tp), Size, - (UpdateRefCount ? " updated" : ""), - HT.isRefCountInf() ? "INF" : std::to_string(HT.getRefCount()).c_str()); + INFO(OMP_INFOTYPE_MAPPING_EXISTS, DeviceID, + "Mapping exists with HstPtrBegin=" DPxMOD ", TgtPtrBegin=" DPxMOD ", " + "Size=%" PRId64 ", RefCount=%s (%s)\n", + DPxPTR(HstPtrBegin), DPxPTR(tp), Size, HT.refCountToStr().c_str(), + RefCountAction); rc = (void *)tp; } else if (PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY) { // If the value isn't found in the mapping and unified shared memory // is on then it means we have stumbled upon a value which we need to // use directly from the host. - DP("Get HstPtrBegin " DPxMOD " Size=%" PRId64 " RefCount=%s\n", - DPxPTR((uintptr_t)HstPtrBegin), Size, - (UpdateRefCount ? " updated" : "")); + DP("Get HstPtrBegin " DPxMOD " Size=%" PRId64 " for unified shared " + "memory\n", + DPxPTR((uintptr_t)HstPtrBegin), Size); IsHostPtr = true; rc = HstPtrBegin; } Index: openmp/libomptarget/test/offloading/info.c =================================================================== --- openmp/libomptarget/test/offloading/info.c +++ openmp/libomptarget/test/offloading/info.c @@ -19,10 +19,10 @@ // INFO: Libomptarget device 0 info: alloc(A[0:64])[256] // INFO: Libomptarget device 0 info: tofrom(B[0:64])[256] // INFO: Libomptarget device 0 info: to(C[0:64])[256] -// INFO: Libomptarget device 0 info: Creating new map entry with HstPtrBegin={{.*}}, TgtPtrBegin={{.*}}, Size=256, Name=A[0:64] -// INFO: Libomptarget device 0 info: Creating new map entry with HstPtrBegin={{.*}}, TgtPtrBegin={{.*}}, Size=256, Name=B[0:64] +// INFO: Libomptarget device 0 info: Creating new map entry with HstPtrBegin={{.*}}, TgtPtrBegin={{.*}}, Size=256, RefCount=1, Name=A[0:64] +// INFO: Libomptarget device 0 info: Creating new map entry with HstPtrBegin={{.*}}, TgtPtrBegin={{.*}}, Size=256, RefCount=1, Name=B[0:64] // INFO: Libomptarget device 0 info: Copying data from host to device, HstPtr={{.*}}, TgtPtr={{.*}}, Size=256, Name=B[0:64] -// INFO: Libomptarget device 0 info: Creating new map entry with HstPtrBegin={{.*}}, TgtPtrBegin={{.*}}, Size=256, Name=C[0:64] +// INFO: Libomptarget device 0 info: Creating new map entry with HstPtrBegin={{.*}}, TgtPtrBegin={{.*}}, Size=256, RefCount=1, Name=C[0:64] // INFO: Libomptarget device 0 info: Copying data from host to device, HstPtr={{.*}}, TgtPtr={{.*}}, Size=256, Name=C[0:64] // INFO: Libomptarget device 0 info: OpenMP Host-Device pointer mappings after block at info.c:{{[0-9]+}}:{{[0-9]+}}: // INFO: Libomptarget device 0 info: Host Ptr Target Ptr Size (B) RefCount Declaration