Skip to content

Commit fc247c8

Browse files
committed
Revert "[OpenMP] Implement TR8 present map type modifier in runtime (2/2)"
This reverts commit 45b8f7e. It attempts to use debug macros `DPxMOD` and `DPxPTR` in release builds. Will fix and reapply later.
1 parent 238bbd4 commit fc247c8

File tree

12 files changed

+42
-390
lines changed

12 files changed

+42
-390
lines changed

openmp/libomptarget/include/omptarget.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@ enum tgt_map_type {
4949
OMP_TGT_MAPTYPE_IMPLICIT = 0x200,
5050
// copy data to device
5151
OMP_TGT_MAPTYPE_CLOSE = 0x400,
52-
// runtime error if not already allocated
53-
OMP_TGT_MAPTYPE_PRESENT = 0x1000,
5452
// member of struct, member given by [16 MSBs] - 1
5553
OMP_TGT_MAPTYPE_MEMBER_OF = 0xffff000000000000
5654
};

openmp/libomptarget/src/device.cpp

Lines changed: 23 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,8 @@ LookupResult DeviceTy::lookupMapping(void *HstPtrBegin, int64_t Size) {
160160
// If NULL is returned, then either data allocation failed or the user tried
161161
// to do an illegal mapping.
162162
void *DeviceTy::getOrAllocTgtPtr(void *HstPtrBegin, void *HstPtrBase,
163-
int64_t Size, bool &IsNew, bool &IsHostPtr,
164-
bool IsImplicit, bool UpdateRefCount,
165-
bool HasCloseModifier,
166-
bool HasPresentModifier) {
163+
int64_t Size, bool &IsNew, bool &IsHostPtr, bool IsImplicit,
164+
bool UpdateRefCount, bool HasCloseModifier) {
167165
void *rc = NULL;
168166
IsHostPtr = false;
169167
IsNew = false;
@@ -192,40 +190,31 @@ void *DeviceTy::getOrAllocTgtPtr(void *HstPtrBegin, void *HstPtrBase,
192190
} else if ((lr.Flags.ExtendsBefore || lr.Flags.ExtendsAfter) && !IsImplicit) {
193191
// Explicit extension of mapped data - not allowed.
194192
DP("Explicit extension of mapping is not allowed.\n");
195-
} else if (RTLs->RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY &&
196-
!HasCloseModifier) {
197-
// If unified shared memory is active, implicitly mapped variables that are
198-
// not privatized use host address. Any explicitly mapped variables also use
199-
// host address where correctness is not impeded. In all other cases maps
200-
// are respected.
201-
// In addition to the mapping rules above, the close map modifier forces the
202-
// mapping of the variable to the device.
203-
if (Size) {
193+
} else if (Size) {
194+
// If unified shared memory is active, implicitly mapped variables that are not
195+
// privatized use host address. Any explicitly mapped variables also use
196+
// host address where correctness is not impeded. In all other cases
197+
// maps are respected.
198+
// In addition to the mapping rules above, the close map
199+
// modifier forces the mapping of the variable to the device.
200+
if (RTLs->RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY &&
201+
!HasCloseModifier) {
204202
DP("Return HstPtrBegin " DPxMOD " Size=%ld RefCount=%s\n",
205-
DPxPTR((uintptr_t)HstPtrBegin), Size,
206-
(UpdateRefCount ? " updated" : ""));
203+
DPxPTR((uintptr_t)HstPtrBegin), Size, (UpdateRefCount ? " updated" : ""));
207204
IsHostPtr = true;
208205
rc = HstPtrBegin;
206+
} else {
207+
// If it is not contained and Size > 0 we should create a new entry for it.
208+
IsNew = true;
209+
uintptr_t tp = (uintptr_t)RTL->data_alloc(RTLDeviceID, Size, HstPtrBegin);
210+
DP("Creating new map entry: HstBase=" DPxMOD ", HstBegin=" DPxMOD ", "
211+
"HstEnd=" DPxMOD ", TgtBegin=" DPxMOD "\n", DPxPTR(HstPtrBase),
212+
DPxPTR(HstPtrBegin), DPxPTR((uintptr_t)HstPtrBegin + Size), DPxPTR(tp));
213+
HostDataToTargetMap.emplace(
214+
HostDataToTargetTy((uintptr_t)HstPtrBase, (uintptr_t)HstPtrBegin,
215+
(uintptr_t)HstPtrBegin + Size, tp));
216+
rc = (void *)tp;
209217
}
210-
} else if (HasPresentModifier) {
211-
DP("Mapping required by 'present' map type modifier does not exist for "
212-
"HstPtrBegin=" DPxMOD ", Size=%ld\n",
213-
DPxPTR(HstPtrBegin), Size);
214-
MESSAGE("device mapping required by 'present' map type modifier does not "
215-
"exist for host address " DPxMOD " (%ld bytes)",
216-
DPxPTR(HstPtrBegin), Size);
217-
} else if (Size) {
218-
// If it is not contained and Size > 0, we should create a new entry for it.
219-
IsNew = true;
220-
uintptr_t tp = (uintptr_t)RTL->data_alloc(RTLDeviceID, Size, HstPtrBegin);
221-
DP("Creating new map entry: HstBase=" DPxMOD ", HstBegin=" DPxMOD ", "
222-
"HstEnd=" DPxMOD ", TgtBegin=" DPxMOD "\n",
223-
DPxPTR(HstPtrBase), DPxPTR(HstPtrBegin),
224-
DPxPTR((uintptr_t)HstPtrBegin + Size), DPxPTR(tp));
225-
HostDataToTargetMap.emplace(
226-
HostDataToTargetTy((uintptr_t)HstPtrBase, (uintptr_t)HstPtrBegin,
227-
(uintptr_t)HstPtrBegin + Size, tp));
228-
rc = (void *)tp;
229218
}
230219

231220
DataMapMtx.unlock();

openmp/libomptarget/src/device.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,8 @@ struct DeviceTy {
177177
uint64_t getMapEntryRefCnt(void *HstPtrBegin);
178178
LookupResult lookupMapping(void *HstPtrBegin, int64_t Size);
179179
void *getOrAllocTgtPtr(void *HstPtrBegin, void *HstPtrBase, int64_t Size,
180-
bool &IsNew, bool &IsHostPtr, bool IsImplicit,
181-
bool UpdateRefCount, bool HasCloseModifier,
182-
bool HasPresentModifier);
180+
bool &IsNew, bool &IsHostPtr, bool IsImplicit, bool UpdateRefCount = true,
181+
bool HasCloseModifier = false);
183182
void *getTgtPtrBegin(void *HstPtrBegin, int64_t Size);
184183
void *getTgtPtrBegin(void *HstPtrBegin, int64_t Size, bool &IsLast,
185184
bool UpdateRefCount, bool &IsHostPtr);

openmp/libomptarget/src/omptarget.cpp

Lines changed: 16 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,6 @@ int target_data_begin(DeviceTy &Device, int32_t arg_num, void **args_base,
308308
// Force the creation of a device side copy of the data when:
309309
// a close map modifier was associated with a map that contained a to.
310310
bool HasCloseModifier = arg_types[i] & OMP_TGT_MAPTYPE_CLOSE;
311-
bool HasPresentModifier = arg_types[i] & OMP_TGT_MAPTYPE_PRESENT;
312311
// UpdateRef is based on MEMBER_OF instead of TARGET_PARAM because if we
313312
// have reached this point via __tgt_target_data_begin and not __tgt_target
314313
// then no argument is marked as TARGET_PARAM ("omp target data map" is not
@@ -317,26 +316,13 @@ int target_data_begin(DeviceTy &Device, int32_t arg_num, void **args_base,
317316
bool UpdateRef = !(arg_types[i] & OMP_TGT_MAPTYPE_MEMBER_OF);
318317
if (arg_types[i] & OMP_TGT_MAPTYPE_PTR_AND_OBJ) {
319318
DP("Has a pointer entry: \n");
320-
// Base is address of pointer.
321-
//
322-
// Usually, the pointer is already allocated by this time. For example:
323-
//
324-
// #pragma omp target map(s.p[0:N])
325-
//
326-
// The map entry for s comes first, and the PTR_AND_OBJ entry comes
327-
// afterward, so the pointer is already allocated by the time the
328-
// PTR_AND_OBJ entry is handled below, and Pointer_TgtPtrBegin is thus
329-
// non-null. However, "declare target link" can produce a PTR_AND_OBJ
330-
// entry for a global that might not already be allocated by the time the
331-
// PTR_AND_OBJ entry is handled below, and so the allocation might fail
332-
// when HasPresentModifier.
333-
Pointer_TgtPtrBegin = Device.getOrAllocTgtPtr(
334-
HstPtrBase, HstPtrBase, sizeof(void *), Pointer_IsNew, IsHostPtr,
335-
IsImplicit, UpdateRef, HasCloseModifier, HasPresentModifier);
319+
// base is address of pointer.
320+
Pointer_TgtPtrBegin = Device.getOrAllocTgtPtr(HstPtrBase, HstPtrBase,
321+
sizeof(void *), Pointer_IsNew, IsHostPtr, IsImplicit, UpdateRef,
322+
HasCloseModifier);
336323
if (!Pointer_TgtPtrBegin) {
337-
DP("Call to getOrAllocTgtPtr returned null pointer (%s).\n",
338-
HasPresentModifier ? "'present' map type modifier"
339-
: "device failure or illegal mapping");
324+
DP("Call to getOrAllocTgtPtr returned null pointer (device failure or "
325+
"illegal mapping).\n");
340326
return OFFLOAD_FAIL;
341327
}
342328
DP("There are %zu bytes allocated at target address " DPxMOD " - is%s new"
@@ -348,15 +334,13 @@ int target_data_begin(DeviceTy &Device, int32_t arg_num, void **args_base,
348334
UpdateRef = true; // subsequently update ref count of pointee
349335
}
350336

351-
void *TgtPtrBegin = Device.getOrAllocTgtPtr(
352-
HstPtrBegin, HstPtrBase, data_size, IsNew, IsHostPtr, IsImplicit,
353-
UpdateRef, HasCloseModifier, HasPresentModifier);
354-
// If data_size==0, then the argument could be a zero-length pointer to
355-
// NULL, so getOrAlloc() returning NULL is not an error.
356-
if (!TgtPtrBegin && (data_size || HasPresentModifier)) {
357-
DP("Call to getOrAllocTgtPtr returned null pointer (%s).\n",
358-
HasPresentModifier ? "'present' map type modifier"
359-
: "device failure or illegal mapping");
337+
void *TgtPtrBegin = Device.getOrAllocTgtPtr(HstPtrBegin, HstPtrBase,
338+
data_size, IsNew, IsHostPtr, IsImplicit, UpdateRef, HasCloseModifier);
339+
if (!TgtPtrBegin && data_size) {
340+
// If data_size==0, then the argument could be a zero-length pointer to
341+
// NULL, so getOrAlloc() returning NULL is not an error.
342+
DP("Call to getOrAllocTgtPtr returned null pointer (device failure or "
343+
"illegal mapping).\n");
360344
return OFFLOAD_FAIL;
361345
}
362346
DP("There are %" PRId64 " bytes allocated at target address " DPxMOD
@@ -475,27 +459,13 @@ int target_data_end(DeviceTy &Device, int32_t arg_num, void **args_base,
475459
(arg_types[i] & OMP_TGT_MAPTYPE_PTR_AND_OBJ);
476460
bool ForceDelete = arg_types[i] & OMP_TGT_MAPTYPE_DELETE;
477461
bool HasCloseModifier = arg_types[i] & OMP_TGT_MAPTYPE_CLOSE;
478-
bool HasPresentModifier = arg_types[i] & OMP_TGT_MAPTYPE_PRESENT;
479462

480463
// If PTR_AND_OBJ, HstPtrBegin is address of pointee
481464
void *TgtPtrBegin = Device.getTgtPtrBegin(HstPtrBegin, data_size, IsLast,
482465
UpdateRef, IsHostPtr);
483-
if (!TgtPtrBegin && (data_size || HasPresentModifier)) {
484-
DP("Mapping does not exist (%s)\n",
485-
(HasPresentModifier ? "'present' map type modifier" : "ignored"));
486-
if (HasPresentModifier) {
487-
// FIXME: This should not be an error on exit from "omp target data",
488-
// but it should be an error upon entering an "omp target exit data".
489-
MESSAGE("device mapping required by 'present' map type modifier does "
490-
"not exist for host address " DPxMOD " (%ld bytes)",
491-
DPxPTR(HstPtrBegin), data_size);
492-
return OFFLOAD_FAIL;
493-
}
494-
} else {
495-
DP("There are %" PRId64 " bytes allocated at target address " DPxMOD
496-
" - is%s last\n",
497-
data_size, DPxPTR(TgtPtrBegin), (IsLast ? "" : " not"));
498-
}
466+
DP("There are %" PRId64 " bytes allocated at target address " DPxMOD
467+
" - is%s last\n", data_size, DPxPTR(TgtPtrBegin),
468+
(IsLast ? "" : " not"));
499469

500470
bool DelEntry = IsLast || ForceDelete;
501471

openmp/libomptarget/src/private.h

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,9 @@ typedef int (*TargetDataFuncPtrTy)(DeviceTy &, int32_t, void **, void **,
8080
int64_t *, int64_t *, void **, __tgt_async_info *);
8181

8282
////////////////////////////////////////////////////////////////////////////////
83-
// implementation for messages
83+
// implementation for fatal messages
8484
////////////////////////////////////////////////////////////////////////////////
8585

86-
#define MESSAGE0(_str) \
87-
do { \
88-
fprintf(stderr, "Libomptarget message: %s\n", _str); \
89-
} while (0)
90-
91-
#define MESSAGE(_str, ...) \
92-
do { \
93-
fprintf(stderr, "Libomptarget message: " _str "\n", __VA_ARGS__); \
94-
} while (0)
95-
9686
#define FATAL_MESSAGE0(_num, _str) \
9787
do { \
9888
fprintf(stderr, "Libomptarget fatal error %d: %s\n", _num, _str); \

openmp/libomptarget/test/mapping/present/target.c

Lines changed: 0 additions & 42 deletions
This file was deleted.

openmp/libomptarget/test/mapping/present/target_data.c

Lines changed: 0 additions & 42 deletions
This file was deleted.

openmp/libomptarget/test/mapping/present/target_enter_data.c

Lines changed: 0 additions & 41 deletions
This file was deleted.

openmp/libomptarget/test/mapping/present/target_exit_data.c

Lines changed: 0 additions & 40 deletions
This file was deleted.

0 commit comments

Comments
 (0)