-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[OpenMP] Adds omp_target_is_accessible routine #138294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
025d36e
b33b27e
bf01578
d20f4d5
cb87242
03d45cc
21b1d6a
95ab6fe
2436211
c92c94f
eeb7604
9d97424
34acf27
d4ecaf6
2792290
712bdd1
dd15747
108e4b9
e9dccd6
4345232
4b51745
0ef2e79
454df9e
79dd36f
0ccae1d
588c394
b751e75
584553e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -196,6 +196,34 @@ EXTERN int omp_target_is_present(const void *Ptr, int DeviceNum) { | |
return Rc; | ||
} | ||
|
||
/// Check whether a pointer is accessible from a device. | ||
/// Returns true when accessibility is guaranteed otherwise returns false. | ||
EXTERN int omp_target_is_accessible(const void *Ptr, size_t Size, | ||
int DeviceNum) { | ||
TIMESCOPE(); | ||
OMPT_IF_BUILT(ReturnAddressSetterRAII RA(__builtin_return_address(0))); | ||
DP("Call to omp_target_is_accessible for device %d, address " DPxMOD | ||
", size %zu\n", | ||
DeviceNum, DPxPTR(Ptr), Size); | ||
|
||
if (!Ptr) { | ||
DP("Call to omp_target_is_accessible with NULL ptr returning false\n"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does spec say so? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does. OMP Specs 6.0, page 607 "If ptr is NULL, the routine returns zero". Please check the specs before asking. |
||
return false; | ||
} | ||
|
||
if (DeviceNum == omp_get_initial_device() || DeviceNum == -1) { | ||
DP("Call to omp_target_is_accessible on host, returning true\n"); | ||
return true; | ||
} | ||
|
||
// The device number must refer to a valid device | ||
auto DeviceOrErr = PM->getDevice(DeviceNum); | ||
if (!DeviceOrErr) | ||
FATAL_MESSAGE(DeviceNum, "%s", toString(DeviceOrErr.takeError()).c_str()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder whether this is a fatal message or simple just return false? What does the spec say when the device number is invalid? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @shiltian : let's return false if the device doesn't exist. A warning is also in order. |
||
|
||
return DeviceOrErr->isAccessiblePtr(Ptr, Size); | ||
} | ||
|
||
EXTERN int omp_target_memcpy(void *Dst, const void *Src, size_t Length, | ||
size_t DstOffset, size_t SrcOffset, int DstDevice, | ||
int SrcDevice) { | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3027,6 +3027,29 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy { | |||||||||||||||
return ((IsAPU || OMPX_ApuMaps) && IsXnackEnabled); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
bool isAccessiblePtrImpl(const void *Ptr, size_t Size) override { | ||||||||||||||||
hsa_amd_pointer_info_t Info; | ||||||||||||||||
Info.size = sizeof(hsa_amd_pointer_info_t); | ||||||||||||||||
|
||||||||||||||||
hsa_agent_t *Agents = nullptr; | ||||||||||||||||
uint32_t Count = 0; | ||||||||||||||||
hsa_status_t Status = | ||||||||||||||||
hsa_amd_pointer_info(Ptr, &Info, malloc, &Count, &Agents); | ||||||||||||||||
|
||||||||||||||||
if (Status != HSA_STATUS_SUCCESS) | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we log here what is the reason of failing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess that the two semantically relevant errors here would be: |
||||||||||||||||
return false; | ||||||||||||||||
|
||||||||||||||||
// Checks if the pointer is known by HSA and accessible by the device | ||||||||||||||||
for (uint32_t i = 0; i < Count; i++) | ||||||||||||||||
if (Agents[i].handle == getAgent().handle) | ||||||||||||||||
return Info.sizeInBytes >= Size; | ||||||||||||||||
Comment on lines
+3043
to
+3045
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
|
||||||||||||||||
|
||||||||||||||||
// If the pointer is unknown to HSA it's assumed a host pointer | ||||||||||||||||
// in that case the device can access it on unified memory support is | ||||||||||||||||
// enabled | ||||||||||||||||
return IsXnackEnabled; | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a nit: there are GPUs that do not have xnack, but still have the ability to access host memory. This behavior is fine for now, but we will have to revisit based on GPU (later, not in this PR). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then it should be documented. |
||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
/// Getters and setters for stack and heap sizes. | ||||||||||||||||
Error getDeviceStackSize(uint64_t &Value) override { | ||||||||||||||||
Value = StackSize; | ||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1069,6 +1069,13 @@ struct GenericDeviceTy : public DeviceAllocatorTy { | |
bool useAutoZeroCopy(); | ||
virtual bool useAutoZeroCopyImpl() { return false; } | ||
|
||
/// Returns true if the plugin can guarantee that the associated | ||
/// storage is accessible | ||
bool isAccessiblePtr(const void *Ptr, size_t Size); | ||
virtual bool isAccessiblePtrImpl(const void *Ptr, size_t Size) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this impl function is only for internal use, can we make them private? |
||
return false; | ||
} | ||
|
||
virtual Expected<omp_interop_val_t *> | ||
createInterop(int32_t InteropType, interop_spec_t &InteropSpec) { | ||
return nullptr; | ||
|
@@ -1499,6 +1506,9 @@ struct GenericPluginTy { | |
/// Returns if the plugin can support automatic copy. | ||
int32_t use_auto_zero_copy(int32_t DeviceId); | ||
|
||
/// Returns if the associated storage is accessible for a given device. | ||
int32_t is_accessible_ptr(int32_t DeviceId, const void *Ptr, size_t Size); | ||
|
||
/// Look up a global symbol in the given binary. | ||
int32_t get_global(__tgt_device_binary Binary, uint64_t Size, | ||
const char *Name, void **DevicePtr); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// RUN: %libomptarget-compilexx-generic | ||
// RUN: env HSA_XNACK=1 %libomptarget-run-generic 2>&1 \ | ||
// RUN: | %fcheck-generic | ||
|
||
// RUN: %libomptarget-compilexx-generic | ||
// RUN: env HSA_XNACK=0 %libomptarget-run-generic 2>&1 \ | ||
// RUN: | %fcheck-generic -check-prefix=NO_USM | ||
|
||
// REQUIRES: unified_shared_memory | ||
// REQUIRES: amdgpu | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd make this a XFAIL for nvptx instead of a requirement, since it always returns false. |
||
|
||
// CHECK: SUCCESS | ||
// NO_USM: Not accessible | ||
|
||
#include <assert.h> | ||
#include <iostream> | ||
#include <omp.h> | ||
#include <stdio.h> | ||
|
||
int main() { | ||
int n = 10000; | ||
int *a = new int[n]; | ||
int err = 0; | ||
|
||
// program must be executed with HSA_XNACK=1 | ||
if (!omp_target_is_accessible(a, n * sizeof(int), /*device_num=*/0)) | ||
printf("Not accessible\n"); | ||
else { | ||
#pragma omp target teams distribute parallel for | ||
for (int i = 0; i < n; i++) | ||
a[i] = i; | ||
|
||
for (int i = 0; i < n; i++) | ||
if (a[i] != i) | ||
err++; | ||
} | ||
|
||
printf("%s\n", err == 0 ? "SUCCESS" : "FAIL"); | ||
return err; | ||
} |
Uh oh!
There was an error while loading. Please reload this page.