Skip to content

[HIP] USM-P2P extension implementation #1234

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

[HIP] USM-P2P extension implementation #1234

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0cdd69f
USM-P2P extension implementation for hip.
JackAKirk Jan 3, 2024
9bd455f
Use camelCase always. Return caught UR error directly.
JackAKirk Jan 9, 2024
ba74c55
Format.
JackAKirk Jan 9, 2024
e3a1b47
Merge branch 'main' into p2p-hip
JackAKirk Feb 2, 2024
12de78a
Merge branch 'main' into p2p-hip
JackAKirk Feb 6, 2024
4217a2d
Revert unrelated cuda changes.
JackAKirk Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions source/adapters/cuda/usm_p2p.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,24 @@

UR_APIEXPORT ur_result_t UR_APICALL urUsmP2PEnablePeerAccessExp(
ur_device_handle_t commandDevice, ur_device_handle_t peerDevice) {

ur_result_t result = UR_RESULT_SUCCESS;
try {
ScopedContext active(commandDevice->getContext());
UR_CHECK_ERROR(cuCtxEnablePeerAccess(peerDevice->getContext(), 0));
} catch (ur_result_t err) {
result = err;
return err;
}
return result;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes appear unrelated. Please submit them as a separate cleanup

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the overhead involved in getting UR PRs merged I would be inclined to make an exception and keep these changes in this PR. They will still appear as a separate commit since we don't squash in UR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a good reason. The hardest bug I ever had to solve was because of "just an extra bit of code" unrelated to the commit message.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right. If we don't squash, then it's slightly better, but it still feels unhygienic to me

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I defer to your judgement

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New PR for those format changes is here: #1311

return UR_RESULT_SUCCESS;
}

UR_APIEXPORT ur_result_t UR_APICALL urUsmP2PDisablePeerAccessExp(
ur_device_handle_t commandDevice, ur_device_handle_t peerDevice) {

ur_result_t result = UR_RESULT_SUCCESS;
try {
ScopedContext active(commandDevice->getContext());
UR_CHECK_ERROR(cuCtxDisablePeerAccess(peerDevice->getContext()));
} catch (ur_result_t err) {
result = err;
return err;
}
return result;
return UR_RESULT_SUCCESS;
}

UR_APIEXPORT ur_result_t UR_APICALL urUsmP2PPeerAccessGetInfoExp(
Expand All @@ -45,16 +41,16 @@ UR_APIEXPORT ur_result_t UR_APICALL urUsmP2PPeerAccessGetInfoExp(
UrReturnHelper ReturnValue(propSize, pPropValue, pPropSizeRet);

int value;
CUdevice_P2PAttribute cu_attr;
CUdevice_P2PAttribute cuAttr;
try {
ScopedContext active(commandDevice->getContext());
switch (propName) {
case UR_EXP_PEER_INFO_UR_PEER_ACCESS_SUPPORTED: {
cu_attr = CU_DEVICE_P2P_ATTRIBUTE_ACCESS_SUPPORTED;
cuAttr = CU_DEVICE_P2P_ATTRIBUTE_ACCESS_SUPPORTED;
break;
}
case UR_EXP_PEER_INFO_UR_PEER_ATOMICS_SUPPORTED: {
cu_attr = CU_DEVICE_P2P_ATTRIBUTE_NATIVE_ATOMIC_SUPPORTED;
cuAttr = CU_DEVICE_P2P_ATTRIBUTE_NATIVE_ATOMIC_SUPPORTED;
break;
}
default: {
Expand All @@ -63,7 +59,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urUsmP2PPeerAccessGetInfoExp(
}

UR_CHECK_ERROR(cuDeviceGetP2PAttribute(
&value, cu_attr, commandDevice->get(), peerDevice->get()));
&value, cuAttr, commandDevice->get(), peerDevice->get()));
} catch (ur_result_t err) {
return err;
}
Expand Down
60 changes: 46 additions & 14 deletions source/adapters/hip/usm_p2p.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,57 @@
//===----------------------------------------------------------------------===//

#include "common.hpp"
#include "context.hpp"

UR_APIEXPORT ur_result_t UR_APICALL
urUsmP2PEnablePeerAccessExp(ur_device_handle_t, ur_device_handle_t) {
detail::ur::die(
"urUsmP2PEnablePeerAccessExp is not implemented for HIP adapter.");
return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
UR_APIEXPORT ur_result_t UR_APICALL urUsmP2PEnablePeerAccessExp(
ur_device_handle_t commandDevice, ur_device_handle_t peerDevice) {
try {
ScopedContext active(commandDevice);
UR_CHECK_ERROR(hipDeviceEnablePeerAccess(peerDevice->get(), 0));
} catch (ur_result_t err) {
return err;
}
return UR_RESULT_SUCCESS;
}

UR_APIEXPORT ur_result_t UR_APICALL
urUsmP2PDisablePeerAccessExp(ur_device_handle_t, ur_device_handle_t) {
detail::ur::die(
"urUsmP2PDisablePeerAccessExp is not implemented for HIP adapter.");
return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
UR_APIEXPORT ur_result_t UR_APICALL urUsmP2PDisablePeerAccessExp(
ur_device_handle_t commandDevice, ur_device_handle_t peerDevice) {
try {
ScopedContext active(commandDevice);
UR_CHECK_ERROR(hipDeviceDisablePeerAccess(peerDevice->get()));
} catch (ur_result_t err) {
return err;
}
return UR_RESULT_SUCCESS;
}

UR_APIEXPORT ur_result_t UR_APICALL urUsmP2PPeerAccessGetInfoExp(
ur_device_handle_t, ur_device_handle_t, ur_exp_peer_info_t, size_t propSize,
void *pPropValue, size_t *pPropSizeRet) {
ur_device_handle_t commandDevice, ur_device_handle_t peerDevice,
ur_exp_peer_info_t propName, size_t propSize, void *pPropValue,
size_t *pPropSizeRet) {
UrReturnHelper ReturnValue(propSize, pPropValue, pPropSizeRet);
// Zero return value indicates that all of the queries currently return false.
return ReturnValue(uint32_t{0});

int value;
hipDeviceP2PAttr hipAttr;
try {
ScopedContext active(commandDevice);
switch (propName) {
case UR_EXP_PEER_INFO_UR_PEER_ACCESS_SUPPORTED: {
hipAttr = hipDevP2PAttrAccessSupported;
break;
}
case UR_EXP_PEER_INFO_UR_PEER_ATOMICS_SUPPORTED: {
hipAttr = hipDevP2PAttrNativeAtomicSupported;
break;
}
default: {
return UR_RESULT_ERROR_INVALID_ENUMERATION;
}
}
UR_CHECK_ERROR(hipDeviceGetP2PAttribute(
&value, hipAttr, commandDevice->get(), peerDevice->get()));
} catch (ur_result_t err) {
return err;
}
return ReturnValue(value);
}