From 85aa991a5d8271afceb36981618105f82d27fc5b Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 17 Feb 2025 15:33:50 -0600 Subject: [PATCH 1/2] [offload] Remove redundant checks in MappingInfoTy::lookupMapping Also add some comments while I was at it. --- offload/libomptarget/OpenMP/Mapping.cpp | 31 +++++++++++-------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/offload/libomptarget/OpenMP/Mapping.cpp b/offload/libomptarget/OpenMP/Mapping.cpp index 4b78ed3360a26..4c21b7184a5c4 100644 --- a/offload/libomptarget/OpenMP/Mapping.cpp +++ b/offload/libomptarget/OpenMP/Mapping.cpp @@ -141,47 +141,44 @@ LookupResult MappingInfoTy::lookupMapping(HDTTMapAccessorTy &HDTTMap, if (HDTTMap->empty()) return LR; + // HDTTMap is std::set, ordered by HstPtrBegin. + // Upper is the first element whose HstPtrBegin > HP. auto Upper = HDTTMap->upper_bound(HP); if (Size == 0) { - // specification v5.1 Pointer Initialization for Device Data Environments - // upper_bound satisfies - // std::prev(upper)->HDTT.HstPtrBegin <= hp < upper->HDTT.HstPtrBegin + // HP satisfies + // std::prev(Upper)->HDTT.HstPtrBegin <= HP < Upper->HDTT.HstPtrBegin if (Upper != HDTTMap->begin()) { LR.TPR.setEntry(std::prev(Upper)->HDTT, OwnedTPR); - // the left side of extended address range is satisfied. - // hp >= LR.TPR.getEntry()->HstPtrBegin || hp >= - // LR.TPR.getEntry()->HstPtrBase - LR.Flags.IsContained = HP < LR.TPR.getEntry()->HstPtrEnd || - HP < LR.TPR.getEntry()->HstPtrBase; + // We know that HP >= LR.TPR.getEntry()->HstPtrBegin + LR.Flags.IsContained = HP < LR.TPR.getEntry()->HstPtrEnd; } if (!LR.Flags.IsContained && Upper != HDTTMap->end()) { LR.TPR.setEntry(Upper->HDTT, OwnedTPR); - // the right side of extended address range is satisfied. - // hp < LR.TPR.getEntry()->HstPtrEnd || hp < LR.TPR.getEntry()->HstPtrBase + // This is a special case: HP is not really contained in a mapped + // region, but it's contained in the extended mapped region, which + // suffices to get the mapping of the base pointer. + // We know that HP < LR.TPR.getEntry()->HstPtrBegin LR.Flags.IsContained = HP >= LR.TPR.getEntry()->HstPtrBase; } } else { - // check the left bin if (Upper != HDTTMap->begin()) { LR.TPR.setEntry(std::prev(Upper)->HDTT, OwnedTPR); - // Is it contained? - LR.Flags.IsContained = HP >= LR.TPR.getEntry()->HstPtrBegin && - HP < LR.TPR.getEntry()->HstPtrEnd && + // We know that HP >= LR.TPR.getEntry()->HstPtrBegin + LR.Flags.IsContained = HP < LR.TPR.getEntry()->HstPtrEnd && (HP + Size) <= LR.TPR.getEntry()->HstPtrEnd; // Does it extend beyond the mapped region? LR.Flags.ExtendsAfter = HP < LR.TPR.getEntry()->HstPtrEnd && (HP + Size) > LR.TPR.getEntry()->HstPtrEnd; } - // check the right bin if (!(LR.Flags.IsContained || LR.Flags.ExtendsAfter) && Upper != HDTTMap->end()) { LR.TPR.setEntry(Upper->HDTT, OwnedTPR); // Does it extend into an already mapped region? - LR.Flags.ExtendsBefore = HP < LR.TPR.getEntry()->HstPtrBegin && - (HP + Size) > LR.TPR.getEntry()->HstPtrBegin; + // We know that HP < LR.TPR.getEntry()->HstPtrBegin + LR.Flags.ExtendsBefore = (HP + Size) > LR.TPR.getEntry()->HstPtrBegin; // Does it extend beyond the mapped region? LR.Flags.ExtendsAfter = HP < LR.TPR.getEntry()->HstPtrEnd && (HP + Size) > LR.TPR.getEntry()->HstPtrEnd; From 10158005741a784bf8748583f7d2abcbbb2f7686 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Tue, 18 Feb 2025 08:25:42 -0600 Subject: [PATCH 2/2] use OpenMP spec terminology in comments --- offload/libomptarget/OpenMP/Mapping.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/offload/libomptarget/OpenMP/Mapping.cpp b/offload/libomptarget/OpenMP/Mapping.cpp index 4c21b7184a5c4..14f5e7dc9d19f 100644 --- a/offload/libomptarget/OpenMP/Mapping.cpp +++ b/offload/libomptarget/OpenMP/Mapping.cpp @@ -156,9 +156,9 @@ LookupResult MappingInfoTy::lookupMapping(HDTTMapAccessorTy &HDTTMap, if (!LR.Flags.IsContained && Upper != HDTTMap->end()) { LR.TPR.setEntry(Upper->HDTT, OwnedTPR); - // This is a special case: HP is not really contained in a mapped - // region, but it's contained in the extended mapped region, which - // suffices to get the mapping of the base pointer. + // This is a special case: HP is not really contained in the mapped + // address range, but it's contained in the extended address range, + // which suffices to get the mapping of the base pointer. // We know that HP < LR.TPR.getEntry()->HstPtrBegin LR.Flags.IsContained = HP >= LR.TPR.getEntry()->HstPtrBase; } @@ -168,7 +168,7 @@ LookupResult MappingInfoTy::lookupMapping(HDTTMapAccessorTy &HDTTMap, // We know that HP >= LR.TPR.getEntry()->HstPtrBegin LR.Flags.IsContained = HP < LR.TPR.getEntry()->HstPtrEnd && (HP + Size) <= LR.TPR.getEntry()->HstPtrEnd; - // Does it extend beyond the mapped region? + // Does it extend beyond the mapped address range? LR.Flags.ExtendsAfter = HP < LR.TPR.getEntry()->HstPtrEnd && (HP + Size) > LR.TPR.getEntry()->HstPtrEnd; } @@ -176,10 +176,10 @@ LookupResult MappingInfoTy::lookupMapping(HDTTMapAccessorTy &HDTTMap, if (!(LR.Flags.IsContained || LR.Flags.ExtendsAfter) && Upper != HDTTMap->end()) { LR.TPR.setEntry(Upper->HDTT, OwnedTPR); - // Does it extend into an already mapped region? + // Does it extend into an already mapped address range? // We know that HP < LR.TPR.getEntry()->HstPtrBegin LR.Flags.ExtendsBefore = (HP + Size) > LR.TPR.getEntry()->HstPtrBegin; - // Does it extend beyond the mapped region? + // Does it extend beyond the mapped address range? LR.Flags.ExtendsAfter = HP < LR.TPR.getEntry()->HstPtrEnd && (HP + Size) > LR.TPR.getEntry()->HstPtrEnd; }