Skip to content
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

[EXP][usm p2p] Add test for ur_exp_usm_p2p and impl for hip #1369

Merged
merged 16 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions include/ur_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -8927,6 +8927,13 @@ urUSMReleaseExp(
#if !defined(__GNUC__)
#pragma region usm p2p(experimental)
#endif
///////////////////////////////////////////////////////////////////////////////
#ifndef UR_USM_P2P_EXTENSION_STRING_EXP
/// @brief The extension string that defines support for USM P2P which is
/// returned when querying device extensions.
#define UR_USM_P2P_EXTENSION_STRING_EXP "ur_exp_usm_p2p"
#endif // UR_USM_P2P_EXTENSION_STRING_EXP

///////////////////////////////////////////////////////////////////////////////
/// @brief Supported peer info
typedef enum ur_exp_peer_info_t {
Expand Down
35 changes: 24 additions & 11 deletions scripts/core/EXP-USM-P2P.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,20 @@ or copying the memory located on a separate "peer" device.

Motivation
--------------------------------------------------------------------------------
Several important projects that the SYCL programming model aims to support use
fine-grained peer to peer memory access controls.
Two such examples that SYCL supports are Pytorch and Gromacs.
This experimental extension to UR aims to provide a portable interface that can
call appropriate driver functions to query and control peer memory access
across the CUDA, HIP and L0 adapters.
Programming models like SYCL or OpenMP aim to support several important
projects that utilise fine-grained peer-to-peer memory access controls.
This experimental extension to the Unified-Runtime API aims to provide a
portable interface that can call appropriate driver functions to query and
control peer memory access within different adapters such as CUDA, HIP and
Level Zero.

Copy link
Contributor

Choose a reason for hiding this comment

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

Minor change to above that you may simply ignore

Programming models like SYCL or OpenMP aim to support several important projects that utilise fine-grained peer-to-peer memory access controls. This experimental extension to the Unified-Runtime API aims to offer a portable interface capable of invoking relevant driver functions to query about and control peer memory access across various adapters, including CUDA, HIP, and Level Zero.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll switch the first sentence, I think this is an improvement. I think the second one is not as good as the original though.

API
--------------------------------------------------------------------------------

Macros
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* ${X}_USM_P2P_EXTENSION_STRING_EXP

Enums
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand All @@ -48,14 +52,23 @@ Functions
* ${x}UsmP2PDisablePeerAccessExp
* ${x}UsmP2PPeerAccessGetInfoExp

Support
--------------------------------------------------------------------------------

Adapters which support this experimental feature *must* return the valid string
defined in ``${X}_USM_P2P_EXTENSION_STRING_EXP`` as one of the options from
${x}DeviceGetInfo when querying for ${X}_DEVICE_INFO_EXTENSIONS.

Changelog
--------------------------------------------------------------------------------

+-----------+------------------------+
| Revision | Changes |
+===========+========================+
| 1.0 | Initial Draft |
+-----------+------------------------+
+-----------+---------------------------------------------+
| Revision | Changes |
+===========+=============================================+
| 1.0 | Initial Draft |
+-----------+---------------------------------------------+
| 1.1 | Added USM_P2P_EXTENSION_STRING_EXP ID Macro |
Copy link
Contributor

@mmoadeli mmoadeli Feb 23, 2024

Choose a reason for hiding this comment

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

not sure if that should be revision 2.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm following other extensions that iterate through 1.1,1.2 etc. This is a minor change to the spec.

+-----------+---------------------------------------------+

Contributors
--------------------------------------------------------------------------------
Expand Down
5 changes: 5 additions & 0 deletions scripts/core/exp-usm-p2p.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ type: header
desc: "Intel $OneApi Unified Runtime Experimental APIs for USM P2P"
ordinal: "99"
--- #--------------------------------------------------------------------------
type: macro
desc: "The extension string that defines support for USM P2P which is returned when querying device extensions."
name: $X_USM_P2P_EXTENSION_STRING_EXP
value: "\"$x_exp_usm_p2p\""
--- #--------------------------------------------------------------------------
type: enum
desc: "Supported peer info"
class: $xUsmP2P
Expand Down
1 change: 1 addition & 0 deletions source/adapters/cuda/device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
SupportedExtensions += "pi_ext_intel_devicelib_assert ";
// Return supported for the UR command-buffer experimental feature
SupportedExtensions += "ur_exp_command_buffer ";
SupportedExtensions += "ur_exp_usm_p2p ";
aarongreig marked this conversation as resolved.
Show resolved Hide resolved
SupportedExtensions += " ";

int Major = 0;
Expand Down
60 changes: 46 additions & 14 deletions source/adapters/hip/usm_p2p.cpp
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);
}
1 change: 1 addition & 0 deletions test/conformance/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ if(UR_DPCXX)
add_subdirectory(program)
add_subdirectory(enqueue)
add_subdirectory(exp_command_buffer)
add_subdirectory(exp_usm_p2p)
else()
message(WARNING
"UR_DPCXX is not defined, the following conformance test executables \
Expand Down
8 changes: 8 additions & 0 deletions test/conformance/exp_usm_p2p/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Copyright (C) 2024 Intel Corporation
# Part of the Unified-Runtime Project, under the Apache License v2.0 with LLVM Exceptions.
# See LICENSE.TXT
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

add_conformance_test_with_devices_environment(exp_usm_p2p
usm_p2p.cpp
)
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
60 changes: 60 additions & 0 deletions test/conformance/exp_usm_p2p/usm_p2p.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright (C) 2024 Intel Corporation
// Part of the Unified-Runtime Project, under the Apache License v2.0 with LLVM
// Exceptions. See LICENSE.TXT
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "uur/fixtures.h"
#include "uur/raii.h"

using urP2PTest = uur::urAllDevicesTest;

TEST_F(urP2PTest, Success) {

if (devices.size() < 2) {
GTEST_SKIP();
}

size_t returned_size;
ASSERT_SUCCESS(urDeviceGetInfo(devices[0], UR_DEVICE_INFO_EXTENSIONS, 0,
nullptr, &returned_size));

std::unique_ptr<char[]> returned_extensions(new char[returned_size]);

ASSERT_SUCCESS(urDeviceGetInfo(devices[0], UR_DEVICE_INFO_EXTENSIONS,
returned_size, returned_extensions.get(),
nullptr));

std::string_view extensions_string(returned_extensions.get());
const bool usm_p2p_support =
extensions_string.find(UR_USM_P2P_EXTENSION_STRING_EXP) !=
std::string::npos;

if (!usm_p2p_support) {
GTEST_SKIP() << "EXP usm p2p feature is not supported.";
}

int value;
ASSERT_SUCCESS(urUsmP2PPeerAccessGetInfoExp(
devices[0], ///< [in] handle of the command device object
devices[1], ///< [in] handle of the peer device object
UR_EXP_PEER_INFO_UR_PEER_ACCESS_SUPPORTED, sizeof(int), &value,
&returned_size));
// Note that whilst it is not currently specified to be a requirement in the
// specification, currently all supported backends return value = 1 for the
// UR_EXP_PEER_INFO_UR_PEER_ACCESS_SUPPORTED query when the query is true
// (matching the native query return values). Generally different backends can
// return different values for a given device query; however it is
// advisable that for boolean queries they return the same values to indicate
// true/false. When this extension is moved out of experimental status such
// boolean return values should be specified by the extension.
ASSERT_EQ(value, 1);

// Just check that this doesn't throw since supporting peer atomics is
// optional and can depend on backend/device.
ASSERT_SUCCESS(urUsmP2PPeerAccessGetInfoExp(
devices[0], devices[1], UR_EXP_PEER_INFO_UR_PEER_ATOMICS_SUPPORTED,
sizeof(int), &value, &returned_size));

ASSERT_SUCCESS(urUsmP2PEnablePeerAccessExp(devices[0], devices[1]));
ASSERT_SUCCESS(urUsmP2PDisablePeerAccessExp(devices[0], devices[1]));
}