Skip to content

Commit f127125

Browse files
committed
[Offload][OpenMP] Support shadow-pointer tracking for Fortran descriptors.
This change adds support for saving full contents of attached Fortran descriptors, and not just their pointee address, in the shadow-pointer table. With this, we now support: * comparing full contents of descriptors to check whether a previous shadow-pointer entry is stale; * restoring the full contents of descriptors With this, we can now use ATTACH map-types for properly mapping Fortran pointer/allocatable arrays, and array-sections on them. e.g.: ```f90 integer, allocatable :: x(:) !$omp target enter data map(to: x(:)) ``` as: ``` void* addr_of_pointee = allocated(x) ? &x(1) : nullptr; void* addr_of_descriptor = &x; int64_t sizeof_pointee = allocated(x) ? sizeof(x(:)) : 0 addr_of_pointee, addr_of_pointee, sizeof_pointee, TO addr_of_descriptor, addr_of_pointee, sizeof(x), ATTACH ```
1 parent 9f7877f commit f127125

File tree

2 files changed

+109
-39
lines changed

2 files changed

+109
-39
lines changed

offload/include/OpenMP/Mapping.h

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,46 @@ class MappingConfig {
4949
/// Information about shadow pointers.
5050
struct ShadowPtrInfoTy {
5151
void **HstPtrAddr = nullptr;
52-
void *HstPtrVal = nullptr;
5352
void **TgtPtrAddr = nullptr;
54-
void *TgtPtrVal = nullptr;
53+
int64_t PtrSize = sizeof(void *); // Size of the pointer/descriptor
54+
55+
// Store the complete contents for both host and target pointers/descriptors.
56+
// 128 bytes is chosen as the "Small" size to cover common Fortran
57+
// descriptors of up to 3 dimensions.
58+
llvm::SmallVector<char, 128> HstPtrContent;
59+
llvm::SmallVector<char, 128> TgtPtrContent;
60+
61+
ShadowPtrInfoTy(void **HstPtrAddr, void **TgtPtrAddr, void *TgtPteeBase,
62+
int64_t PtrSize)
63+
: HstPtrAddr(HstPtrAddr), TgtPtrAddr(TgtPtrAddr), PtrSize(PtrSize),
64+
HstPtrContent(PtrSize), TgtPtrContent(PtrSize) {
65+
constexpr int64_t VoidPtrSize = sizeof(void *);
66+
assert(HstPtrAddr != nullptr && "HstPtrAddr is nullptr");
67+
assert(TgtPtrAddr != nullptr && "TgtPtrAddr is nullptr");
68+
assert(PtrSize >= VoidPtrSize && "PtrSize is less than sizeof(void *)");
69+
70+
void *HstPteeBase = *HstPtrAddr;
71+
// The first VoidPtrSize bytes for HstPtrContent/TgtPtrContent are from
72+
// HstPteeBase/TgtPteeBase.
73+
std::memcpy(HstPtrContent.data(), &HstPteeBase, VoidPtrSize);
74+
std::memcpy(TgtPtrContent.data(), &TgtPteeBase, VoidPtrSize);
75+
76+
// If we are not dealing with Fortran descriptors (pointers larger than
77+
// VoidPtrSize), then that's that.
78+
if (PtrSize <= VoidPtrSize)
79+
return;
80+
81+
// For larger pointers, i.e. Fortran descriptors, the remaining contents of
82+
// the descriptor come from the host descriptor, i.e. HstPtrAddr.
83+
std::memcpy(HstPtrContent.data() + VoidPtrSize,
84+
reinterpret_cast<char *>(HstPtrAddr) + VoidPtrSize,
85+
PtrSize - VoidPtrSize);
86+
std::memcpy(TgtPtrContent.data() + VoidPtrSize,
87+
reinterpret_cast<char *>(TgtPtrAddr) + VoidPtrSize,
88+
PtrSize - VoidPtrSize);
89+
}
90+
91+
ShadowPtrInfoTy() = delete;
5592

5693
bool operator==(const ShadowPtrInfoTy &Other) const {
5794
return HstPtrAddr == Other.HstPtrAddr;
@@ -243,9 +280,25 @@ struct HostDataToTargetTy {
243280
auto Pair = States->ShadowPtrInfos.insert(ShadowPtrInfo);
244281
if (Pair.second)
245282
return true;
283+
246284
// Check for a stale entry, if found, replace the old one.
247-
if ((*Pair.first).TgtPtrVal == ShadowPtrInfo.TgtPtrVal)
285+
286+
// For Fortran descriptors, we need to compare their full contents,
287+
// as the starting address may be the same while other fields have
288+
// been updated. e.g.
289+
//
290+
// !$omp target enter data map(x(1:100)) ! (1)
291+
// p => x(10: 19)
292+
// !$omp target enter data map(p, p(:)) ! (2)
293+
// p => x(5: 9)
294+
// !$omp target enter data map(attach(always): p(:)) ! (3)
295+
//
296+
// While &desc_p and &p(1) (TgtPtrAddr and first "sizeof(void*)" bytes of
297+
// TgtPtrContent) are same for (2) and (3), the pointer attachment for (3)
298+
// needs to update the bounds information in the descriptor of p on device.
299+
if ((*Pair.first).TgtPtrContent == ShadowPtrInfo.TgtPtrContent)
248300
return false;
301+
249302
States->ShadowPtrInfos.erase(ShadowPtrInfo);
250303
return addShadowPointer(ShadowPtrInfo);
251304
}

offload/libomptarget/omptarget.cpp

Lines changed: 53 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535

3636
#include <cassert>
3737
#include <cstdint>
38-
#include <vector>
3938

4039
using llvm::SmallVector;
4140
#ifdef OMPT_SUPPORT
@@ -396,7 +395,7 @@ static int performPointerAttachment(DeviceTy &Device, AsyncInfoTy &AsyncInfo,
396395
assert(PtrTPR.getEntry() &&
397396
"Need a valid pointer entry to perform pointer-attachment");
398397

399-
int64_t VoidPtrSize = sizeof(void *);
398+
constexpr int64_t VoidPtrSize = sizeof(void *);
400399
assert(HstPtrSize >= VoidPtrSize && "PointerSize is too small");
401400

402401
uint64_t Delta = reinterpret_cast<uint64_t>(HstPteeBegin) -
@@ -411,23 +410,8 @@ static int performPointerAttachment(DeviceTy &Device, AsyncInfoTy &AsyncInfo,
411410
DPxPTR(TgtPteeBase), DPxPTR(TgtPteeBegin));
412411

413412
// Add shadow pointer tracking
414-
// TODO: Support shadow-tracking of larger than VoidPtrSize pointers,
415-
// to support restoration of Fortran descriptors. Currently, this check
416-
// would return false, even if the host Fortran descriptor had been
417-
// updated since its previous map, and we should have updated its
418-
// device counterpart. e.g.
419-
//
420-
// !$omp target enter data map(x(1:100)) ! (1)
421-
// p => x(10: 19)
422-
// !$omp target enter data map(p, p(:)) ! (2)
423-
// p => x(5: 9)
424-
// !$omp target enter data map(attach(always): p(:)) ! (3)
425-
//
426-
// While PtrAddr(&desc_p) and PteeBase(&p(1)) are same for (2) and (3), the
427-
// pointer attachment for (3) needs to update the bounds information
428-
// in the descriptor of p on device.
429413
if (!PtrTPR.getEntry()->addShadowPointer(
430-
ShadowPtrInfoTy{HstPtrAddr, HstPteeBase, TgtPtrAddr, TgtPteeBase})) {
414+
ShadowPtrInfoTy{HstPtrAddr, TgtPtrAddr, TgtPteeBase, HstPtrSize})) {
431415
DP("Pointer " DPxMOD " is already attached to " DPxMOD "\n",
432416
DPxPTR(TgtPtrAddr), DPxPTR(TgtPteeBase));
433417
return OFFLOAD_SUCCESS;
@@ -940,17 +924,29 @@ postProcessingTargetDataEnd(DeviceTy *Device,
940924
DelEntry = false;
941925
}
942926

943-
// If we copied back to the host a struct/array containing pointers,
944-
// we need to restore the original host pointer values from their
945-
// shadow copies. If the struct is going to be deallocated, remove any
946-
// remaining shadow pointer entries for this struct.
927+
// If we copied back to the host a struct/array containing pointers, or
928+
// Fortran descriptors (which are larger than a "void *"), we need to
929+
// restore the original host pointer/descriptor values from their shadow
930+
// copies. If the struct is going to be deallocated, remove any remaining
931+
// shadow pointer entries for this struct.
947932
const bool HasFrom = ArgType & OMP_TGT_MAPTYPE_FROM;
948933
if (HasFrom) {
949934
Entry->foreachShadowPointerInfo([&](const ShadowPtrInfoTy &ShadowPtr) {
950-
*ShadowPtr.HstPtrAddr = ShadowPtr.HstPtrVal;
951-
DP("Restoring original host pointer value " DPxMOD " for host "
952-
"pointer " DPxMOD "\n",
953-
DPxPTR(ShadowPtr.HstPtrVal), DPxPTR(ShadowPtr.HstPtrAddr));
935+
constexpr int64_t VoidPtrSize = sizeof(void *);
936+
if (ShadowPtr.PtrSize > VoidPtrSize) {
937+
DP("Restoring host descriptor " DPxMOD
938+
" to its original content (%" PRId64
939+
" bytes), containing pointee address " DPxMOD "\n",
940+
DPxPTR(ShadowPtr.HstPtrAddr), ShadowPtr.PtrSize,
941+
DPxPTR(ShadowPtr.HstPtrContent.data()));
942+
} else {
943+
DP("Restoring host pointer " DPxMOD " to its original value " DPxMOD
944+
"\n",
945+
DPxPTR(ShadowPtr.HstPtrAddr),
946+
DPxPTR(ShadowPtr.HstPtrContent.data()));
947+
}
948+
std::memcpy(ShadowPtr.HstPtrAddr, ShadowPtr.HstPtrContent.data(),
949+
ShadowPtr.PtrSize);
954950
return OFFLOAD_SUCCESS;
955951
});
956952
}
@@ -1163,12 +1159,22 @@ static int targetDataContiguous(ident_t *Loc, DeviceTy &Device, void *ArgsBase,
11631159
if (TPR.getEntry()) {
11641160
int Ret = TPR.getEntry()->foreachShadowPointerInfo(
11651161
[&](ShadowPtrInfoTy &ShadowPtr) {
1166-
DP("Restoring original target pointer value " DPxMOD " for target "
1167-
"pointer " DPxMOD "\n",
1168-
DPxPTR(ShadowPtr.TgtPtrVal), DPxPTR(ShadowPtr.TgtPtrAddr));
1162+
constexpr int64_t VoidPtrSize = sizeof(void *);
1163+
if (ShadowPtr.PtrSize > VoidPtrSize) {
1164+
DP("Restoring target descriptor " DPxMOD
1165+
" to its original content (%" PRId64
1166+
" bytes), containing pointee address " DPxMOD "\n",
1167+
DPxPTR(ShadowPtr.TgtPtrAddr), ShadowPtr.PtrSize,
1168+
DPxPTR(ShadowPtr.TgtPtrContent.data()));
1169+
} else {
1170+
DP("Restoring target pointer " DPxMOD
1171+
" to its original value " DPxMOD "\n",
1172+
DPxPTR(ShadowPtr.TgtPtrAddr),
1173+
DPxPTR(ShadowPtr.TgtPtrContent.data()));
1174+
}
11691175
Ret = Device.submitData(ShadowPtr.TgtPtrAddr,
1170-
(void *)&ShadowPtr.TgtPtrVal,
1171-
sizeof(void *), AsyncInfo);
1176+
ShadowPtr.TgtPtrContent.data(),
1177+
ShadowPtr.PtrSize, AsyncInfo);
11721178
if (Ret != OFFLOAD_SUCCESS) {
11731179
REPORT("Copying data to device failed.\n");
11741180
return OFFLOAD_FAIL;
@@ -1193,15 +1199,26 @@ static int targetDataContiguous(ident_t *Loc, DeviceTy &Device, void *ArgsBase,
11931199
}
11941200

11951201
// Wait for device-to-host memcopies for whole struct to complete,
1196-
// before restoring the correct host pointer.
1202+
// before restoring the correct host pointer/descriptor.
11971203
if (auto *Entry = TPR.getEntry()) {
11981204
AsyncInfo.addPostProcessingFunction([=]() -> int {
11991205
int Ret = Entry->foreachShadowPointerInfo(
12001206
[&](const ShadowPtrInfoTy &ShadowPtr) {
1201-
*ShadowPtr.HstPtrAddr = ShadowPtr.HstPtrVal;
1202-
DP("Restoring original host pointer value " DPxMOD
1203-
" for host pointer " DPxMOD "\n",
1204-
DPxPTR(ShadowPtr.HstPtrVal), DPxPTR(ShadowPtr.HstPtrAddr));
1207+
constexpr int64_t VoidPtrSize = sizeof(void *);
1208+
if (ShadowPtr.PtrSize > VoidPtrSize) {
1209+
DP("Restoring host descriptor " DPxMOD
1210+
" to its original content (%" PRId64
1211+
" bytes), containing pointee address " DPxMOD "\n",
1212+
DPxPTR(ShadowPtr.HstPtrAddr), ShadowPtr.PtrSize,
1213+
DPxPTR(ShadowPtr.HstPtrContent.data()));
1214+
} else {
1215+
DP("Restoring host pointer " DPxMOD
1216+
" to its original value " DPxMOD "\n",
1217+
DPxPTR(ShadowPtr.HstPtrAddr),
1218+
DPxPTR(ShadowPtr.HstPtrContent.data()));
1219+
}
1220+
std::memcpy(ShadowPtr.HstPtrAddr, ShadowPtr.HstPtrContent.data(),
1221+
ShadowPtr.PtrSize);
12051222
return OFFLOAD_SUCCESS;
12061223
});
12071224
Entry->unlock();

0 commit comments

Comments
 (0)