Skip to content

Commit 357ccd9

Browse files
committed
[OpenMP] Changes error handling for omp_target_is_accessible
Addresses rewiev comments. Changes LIT test to XFAIL on nvptx. Changes implementation to provide better debug information to the user about failures.
1 parent 588c394 commit 357ccd9

File tree

4 files changed

+21
-10
lines changed

4 files changed

+21
-10
lines changed

offload/plugins-nextgen/amdgpu/src/rtl.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3027,7 +3027,7 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy {
30273027
return ((IsAPU || OMPX_ApuMaps) && IsXnackEnabled);
30283028
}
30293029

3030-
bool isAccessiblePtrImpl(const void *Ptr, size_t Size) override {
3030+
Expected<bool> isAccessiblePtrImpl(const void *Ptr, size_t Size) override {
30313031
hsa_amd_pointer_info_t Info;
30323032
Info.size = sizeof(hsa_amd_pointer_info_t);
30333033

@@ -3036,8 +3036,8 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy {
30363036
hsa_status_t Status =
30373037
hsa_amd_pointer_info(Ptr, &Info, malloc, &Count, &Agents);
30383038

3039-
if (Status != HSA_STATUS_SUCCESS)
3040-
return false;
3039+
if (auto Err = Plugin::check(Status, "error in hsa_amd_pointer_info: %s"))
3040+
return std::move(Err);
30413041

30423042
// Checks if the pointer is known by HSA and accessible by the device
30433043
for (uint32_t i = 0; i < Count; i++)

offload/plugins-nextgen/common/include/PluginInterface.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,10 +1071,7 @@ struct GenericDeviceTy : public DeviceAllocatorTy {
10711071

10721072
/// Returns true if the plugin can guarantee that the associated
10731073
/// storage is accessible
1074-
bool isAccessiblePtr(const void *Ptr, size_t Size);
1075-
virtual bool isAccessiblePtrImpl(const void *Ptr, size_t Size) {
1076-
return false;
1077-
}
1074+
Expected<bool> isAccessiblePtr(const void *Ptr, size_t Size);
10781075

10791076
virtual Expected<omp_interop_val_t *>
10801077
createInterop(int32_t InteropType, interop_spec_t &InteropSpec) {
@@ -1176,6 +1173,10 @@ struct GenericDeviceTy : public DeviceAllocatorTy {
11761173
/// Per device setting of MemoryManager's Threshold
11771174
virtual size_t getMemoryManagerSizeThreshold() { return 0; }
11781175

1176+
virtual Expected<bool> isAccessiblePtrImpl(const void *Ptr, size_t Size) {
1177+
return false;
1178+
}
1179+
11791180
/// Environment variables defined by the OpenMP standard.
11801181
Int32Envar OMP_TeamLimit;
11811182
Int32Envar OMP_NumTeams;

offload/plugins-nextgen/common/src/PluginInterface.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1593,7 +1593,7 @@ Error GenericDeviceTy::syncEvent(void *EventPtr) {
15931593

15941594
bool GenericDeviceTy::useAutoZeroCopy() { return useAutoZeroCopyImpl(); }
15951595

1596-
bool GenericDeviceTy::isAccessiblePtr(const void *Ptr, size_t Size) {
1596+
Expected<bool> GenericDeviceTy::isAccessiblePtr(const void *Ptr, size_t Size) {
15971597
return isAccessiblePtrImpl(Ptr, Size);
15981598
}
15991599

@@ -2153,7 +2153,17 @@ int32_t GenericPluginTy::use_auto_zero_copy(int32_t DeviceId) {
21532153

21542154
int32_t GenericPluginTy::is_accessible_ptr(int32_t DeviceId, const void *Ptr,
21552155
size_t Size) {
2156-
return getDevice(DeviceId).isAccessiblePtr(Ptr, Size);
2156+
auto HandleError = [&](Error Err) -> bool {
2157+
[[maybe_unused]] std::string ErrStr = toString(std::move(Err));
2158+
DP("Failure while checking accessibility of pointer %p for device %d: %s", Ptr, DeviceId, ErrStr.c_str());
2159+
return false;
2160+
};
2161+
2162+
auto AccessibleOrErr = getDevice(DeviceId).isAccessiblePtr(Ptr, Size);
2163+
if (Error Err = AccessibleOrErr.takeError())
2164+
return HandleError(std::move(Err));
2165+
2166+
return *AccessibleOrErr;
21572167
}
21582168

21592169
int32_t GenericPluginTy::get_global(__tgt_device_binary Binary, uint64_t Size,

offload/test/mapping/is_accessible.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
// RUN: | %fcheck-generic -check-prefix=NO_USM
88

99
// REQUIRES: unified_shared_memory
10-
// REQUIRES: amdgpu
10+
// XFAIL: nvptx
1111

1212
// CHECK: SUCCESS
1313
// NO_USM: Not accessible

0 commit comments

Comments
 (0)