Skip to content

Commit f589b38

Browse files
ronliebronlieb
committed
Revert "[Offload][OpenMP] Support shadow-pointer tracking for Fortran descriptors. (llvm#158370)"
This reverts commit 5af3fa8.
1 parent de80b87 commit f589b38

File tree

3 files changed

+51
-114
lines changed

3 files changed

+51
-114
lines changed

‎offload/include/OpenMP/Mapping.h‎

Lines changed: 3 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -49,46 +49,9 @@ class MappingConfig {
4949
/// Information about shadow pointers.
5050
struct ShadowPtrInfoTy {
5151
void **HstPtrAddr = nullptr;
52+
void *HstPtrVal = nullptr;
5253
void **TgtPtrAddr = 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-
// 96 bytes is chosen as the "Small" size to cover simple Fortran
57-
// descriptors of up to 3 dimensions.
58-
llvm::SmallVector<char, 96> HstPtrContent;
59-
llvm::SmallVector<char, 96> 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 *>(HstPtrAddr) + VoidPtrSize,
88-
PtrSize - VoidPtrSize);
89-
}
90-
91-
ShadowPtrInfoTy() = delete;
54+
void *TgtPtrVal = nullptr;
9255

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

‎offload/libomptarget/omptarget.cpp‎

Lines changed: 46 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ static int performPointerAttachment(DeviceTy &Device, AsyncInfoTy &AsyncInfo,
407407
assert(PtrTPR.getEntry() &&
408408
"Need a valid pointer entry to perform pointer-attachment");
409409

410-
constexpr int64_t VoidPtrSize = sizeof(void *);
410+
int64_t VoidPtrSize = sizeof(void *);
411411
assert(HstPtrSize >= VoidPtrSize && "PointerSize is too small");
412412

413413
uint64_t Delta = reinterpret_cast<uint64_t>(HstPteeBegin) -
@@ -422,8 +422,23 @@ static int performPointerAttachment(DeviceTy &Device, AsyncInfoTy &AsyncInfo,
422422
DPxPTR(TgtPteeBase), DPxPTR(TgtPteeBegin));
423423

424424
// Add shadow pointer tracking
425+
// TODO: Support shadow-tracking of larger than VoidPtrSize pointers,
426+
// to support restoration of Fortran descriptors. Currently, this check
427+
// would return false, even if the host Fortran descriptor had been
428+
// updated since its previous map, and we should have updated its
429+
// device counterpart. e.g.
430+
//
431+
// !$omp target enter data map(x(1:100)) ! (1)
432+
// p => x(10: 19)
433+
// !$omp target enter data map(p, p(:)) ! (2)
434+
// p => x(5: 9)
435+
// !$omp target enter data map(attach(always): p(:)) ! (3)
436+
//
437+
// While PtrAddr(&desc_p) and PteeBase(&p(1)) are same for (2) and (3), the
438+
// pointer attachment for (3) needs to update the bounds information
439+
// in the descriptor of p on device.
425440
if (!PtrTPR.getEntry()->addShadowPointer(
426-
ShadowPtrInfoTy{HstPtrAddr, TgtPtrAddr, TgtPteeBase, HstPtrSize})) {
441+
ShadowPtrInfoTy{HstPtrAddr, HstPteeBase, TgtPtrAddr, TgtPteeBase})) {
427442
DP("Pointer " DPxMOD " is already attached to " DPxMOD "\n",
428443
DPxPTR(TgtPtrAddr), DPxPTR(TgtPteeBase));
429444
return OFFLOAD_SUCCESS;
@@ -954,29 +969,22 @@ postProcessingTargetDataEnd(DeviceTy *Device,
954969
DelEntry = false;
955970
}
956971

957-
// If we copied back to the host a struct/array containing pointers, or
958-
// Fortran descriptors (which are larger than a "void *"), we need to
959-
// restore the original host pointer/descriptor values from their shadow
960-
// copies. If the struct is going to be deallocated, remove any remaining
961-
// shadow pointer entries for this struct.
972+
// If we copied back to the host a struct/array containing pointers,
973+
// we need to restore the original host pointer values from their
974+
// shadow copies. If the struct is going to be deallocated, remove any
975+
// remaining shadow pointer entries for this struct.
962976
const bool HasFrom = ArgType & OMP_TGT_MAPTYPE_FROM;
963977
if (HasFrom) {
964978
Entry->foreachShadowPointerInfo([&](const ShadowPtrInfoTy &ShadowPtr) {
965-
constexpr int64_t VoidPtrSize = sizeof(void *);
966-
if (ShadowPtr.PtrSize > VoidPtrSize) {
967-
DP("Restoring host descriptor " DPxMOD
968-
" to its original content (%" PRId64
969-
" bytes), containing pointee address " DPxMOD "\n",
970-
DPxPTR(ShadowPtr.HstPtrAddr), ShadowPtr.PtrSize,
971-
DPxPTR(ShadowPtr.HstPtrContent.data()));
972-
} else {
973-
DP("Restoring host pointer " DPxMOD " to its original value " DPxMOD
974-
"\n",
975-
DPxPTR(ShadowPtr.HstPtrAddr),
976-
DPxPTR(ShadowPtr.HstPtrContent.data()));
977-
}
978-
std::memcpy(ShadowPtr.HstPtrAddr, ShadowPtr.HstPtrContent.data(),
979-
ShadowPtr.PtrSize);
979+
const bool isZeroCopy = PM->getRequirements() & OMPX_REQ_AUTO_ZERO_COPY;
980+
const bool isUSMMode =
981+
PM->getRequirements() & OMP_REQ_UNIFIED_SHARED_MEMORY;
982+
if (*ShadowPtr.HstPtrAddr == nullptr || isZeroCopy || isUSMMode)
983+
return OFFLOAD_SUCCESS;
984+
*ShadowPtr.HstPtrAddr = ShadowPtr.HstPtrVal;
985+
DP("Restoring original host pointer value " DPxMOD " for host "
986+
"pointer " DPxMOD "\n",
987+
DPxPTR(ShadowPtr.HstPtrVal), DPxPTR(ShadowPtr.HstPtrAddr));
980988
return OFFLOAD_SUCCESS;
981989
});
982990
}
@@ -1189,22 +1197,12 @@ static int targetDataContiguous(ident_t *Loc, DeviceTy &Device, void *ArgsBase,
11891197
if (TPR.getEntry()) {
11901198
int Ret = TPR.getEntry()->foreachShadowPointerInfo(
11911199
[&](ShadowPtrInfoTy &ShadowPtr) {
1192-
constexpr int64_t VoidPtrSize = sizeof(void *);
1193-
if (ShadowPtr.PtrSize > VoidPtrSize) {
1194-
DP("Restoring target descriptor " DPxMOD
1195-
" to its original content (%" PRId64
1196-
" bytes), containing pointee address " DPxMOD "\n",
1197-
DPxPTR(ShadowPtr.TgtPtrAddr), ShadowPtr.PtrSize,
1198-
DPxPTR(ShadowPtr.TgtPtrContent.data()));
1199-
} else {
1200-
DP("Restoring target pointer " DPxMOD
1201-
" to its original value " DPxMOD "\n",
1202-
DPxPTR(ShadowPtr.TgtPtrAddr),
1203-
DPxPTR(ShadowPtr.TgtPtrContent.data()));
1204-
}
1200+
DP("Restoring original target pointer value " DPxMOD " for target "
1201+
"pointer " DPxMOD "\n",
1202+
DPxPTR(ShadowPtr.TgtPtrVal), DPxPTR(ShadowPtr.TgtPtrAddr));
12051203
Ret = Device.submitData(ShadowPtr.TgtPtrAddr,
1206-
ShadowPtr.TgtPtrContent.data(),
1207-
ShadowPtr.PtrSize, AsyncInfo);
1204+
(void *)&ShadowPtr.TgtPtrVal,
1205+
sizeof(void *), AsyncInfo);
12081206
if (Ret != OFFLOAD_SUCCESS) {
12091207
REPORT("Copying data to device failed.\n");
12101208
return OFFLOAD_FAIL;
@@ -1229,26 +1227,21 @@ static int targetDataContiguous(ident_t *Loc, DeviceTy &Device, void *ArgsBase,
12291227
}
12301228

12311229
// Wait for device-to-host memcopies for whole struct to complete,
1232-
// before restoring the correct host pointer/descriptor.
1230+
// before restoring the correct host pointer.
12331231
if (auto *Entry = TPR.getEntry()) {
12341232
AsyncInfo.addPostProcessingFunction([=]() -> int {
12351233
int Ret = Entry->foreachShadowPointerInfo(
12361234
[&](const ShadowPtrInfoTy &ShadowPtr) {
1237-
constexpr int64_t VoidPtrSize = sizeof(void *);
1238-
if (ShadowPtr.PtrSize > VoidPtrSize) {
1239-
DP("Restoring host descriptor " DPxMOD
1240-
" to its original content (%" PRId64
1241-
" bytes), containing pointee address " DPxMOD "\n",
1242-
DPxPTR(ShadowPtr.HstPtrAddr), ShadowPtr.PtrSize,
1243-
DPxPTR(ShadowPtr.HstPtrContent.data()));
1244-
} else {
1245-
DP("Restoring host pointer " DPxMOD
1246-
" to its original value " DPxMOD "\n",
1247-
DPxPTR(ShadowPtr.HstPtrAddr),
1248-
DPxPTR(ShadowPtr.HstPtrContent.data()));
1249-
}
1250-
std::memcpy(ShadowPtr.HstPtrAddr, ShadowPtr.HstPtrContent.data(),
1251-
ShadowPtr.PtrSize);
1235+
const bool isZeroCopy =
1236+
PM->getRequirements() & OMPX_REQ_AUTO_ZERO_COPY;
1237+
const bool isUSMMode =
1238+
PM->getRequirements() & OMP_REQ_UNIFIED_SHARED_MEMORY;
1239+
if (*ShadowPtr.HstPtrAddr == nullptr || isZeroCopy || isUSMMode)
1240+
return OFFLOAD_SUCCESS;
1241+
*ShadowPtr.HstPtrAddr = ShadowPtr.HstPtrVal;
1242+
DP("Restoring original host pointer value " DPxMOD
1243+
" for host pointer " DPxMOD "\n",
1244+
DPxPTR(ShadowPtr.HstPtrVal), DPxPTR(ShadowPtr.HstPtrAddr));
12521245
return OFFLOAD_SUCCESS;
12531246
});
12541247
Entry->unlock();

‎revert_patches.txt‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@ d57230c7 [AMDGPU][MC] Disallow op_sel in some VOP3P dot instructions (#100485)
1515
Revert "[SLP]Support LShr as base for copyable elements"
1616
breaks build rocPRIM
1717
---
18-
breaks offload build
19-
209d91d9e4c4 [Offload] Fix CHECK string in llvm-omp-device-info test (#156872)
20-
---
21-
breaks build flang nekbone
22-
[flang] Attach proper storage to [hl]fir.declare in lowering. (#155742)
18+
build issues
19+
5af3fa81cc12 [Offload][OpenMP] Support shadow-pointer tracking
2320
---

0 commit comments

Comments
 (0)