-
Notifications
You must be signed in to change notification settings - Fork 743
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
[SYCL][CUDA] Implement sycl_ext_oneapi_peer_access extension #8303
Changes from 87 commits
5d788c6
8685475
de16f88
b5f9481
64ecf25
15d4bf6
a35294f
df55a69
ddca3c3
c3a2009
f0f448d
1855367
644c880
e5b421e
6f45d53
1849619
aa7a7eb
3380230
5f6360c
e33a578
32d714a
7d12cd7
4cf5999
63f23e5
aaec286
76245f2
55a9b6a
80dfb3a
76f6772
85949f7
65aa452
ea7e855
eae9b12
c825b9c
ae358bb
ce81826
72aca2e
c2579aa
0c87405
dfcb01c
b84da4d
9e5408b
57c5747
029c1fe
8d41240
b86954d
a6f7602
cc5eb63
716bf30
484cf25
333b7d6
308f45a
8b8cbc8
b2b6fcb
2bea80b
04b8610
d0b138c
3e488c4
c1905c6
19d2a7d
f2a2026
37aecb1
e80ff9b
8956f50
b987dba
6724a40
69a6506
11ca2a6
8b9c2ab
0a09023
f1eb0f0
40e93d0
e5e967f
92e1420
f0993b9
28028ac
6ed3213
512d2a8
1e088a1
80dd26a
9dc099c
b191914
5d83aeb
c389980
8bd6b60
5e7d821
ab3ac25
47acd23
4ab6215
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 | ||||
---|---|---|---|---|---|---|
|
@@ -36,18 +36,19 @@ https://github.com/intel/llvm/issues | |||||
|
||||||
== Dependencies | ||||||
|
||||||
This extension is written against the SYCL 2020 revision 6 specification. All | ||||||
This extension is written against the SYCL 2020 revision 7 specification. All | ||||||
references below to the "core SYCL specification" or to section numbers in the | ||||||
SYCL specification refer to that revision. | ||||||
|
||||||
== Status | ||||||
|
||||||
This is a proposed extension specification, intended to gather community | ||||||
feedback. Interfaces defined in this specification may not be implemented yet | ||||||
or may be in a preliminary state. The specification itself may also change in | ||||||
incompatible ways before it is finalized. *Shipping software products should | ||||||
not rely on APIs defined in this specification.* | ||||||
This extension is implemented and fully supported by DPC++. | ||||||
|
||||||
== Backend support status | ||||||
|
||||||
This extension is currently implemented in DPC++ for all GPU devices and | ||||||
backends, however, only the CUDA backend allows peer to peer memory access. | ||||||
Other backends report false from the ext_oneapi_can_access_peer query. | ||||||
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.
Suggested change
Code font is better here. |
||||||
|
||||||
== Overview | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,9 +97,11 @@ | |
// 14.33 Added new parameter (memory object properties) to | ||
// piextKernelSetArgMemObj | ||
// 14.34 Added command-buffer extension methods | ||
// 14.35 Added piextEnablePeerAccess, piextDisablePeerAccess, | ||
// piextPeerAccessGetInfo, and pi_peer_attr enum. | ||
|
||
#define _PI_H_VERSION_MAJOR 14 | ||
#define _PI_H_VERSION_MINOR 34 | ||
#define _PI_H_VERSION_MINOR 35 | ||
|
||
#define _PI_STRING_HELPER(a) #a | ||
#define _PI_CONCAT(a, b) _PI_STRING_HELPER(a.b) | ||
|
@@ -1029,7 +1031,17 @@ using pi_image_desc = _pi_image_desc; | |
|
||
typedef enum { PI_MEM_CONTEXT = 0x1106, PI_MEM_SIZE = 0x1102 } _pi_mem_info; | ||
|
||
typedef enum { | ||
PI_PEER_ACCESS_SUPPORTED = | ||
0x0, ///< returns a uint32_t: 1 if P2P Access is supported | ||
///< otherwise P2P Access is not supported. | ||
PI_PEER_ATOMICS_SUPPORTED = | ||
0x1 ///< returns a uint32_t: 1 if Atomic operations are supported over the | ||
///< P2P link, otherwise such operations are not supported. | ||
} _pi_peer_attr; | ||
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. we need to define the returned value (and type) for these queries, is it pi_bool aka uint32_t? 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. See the definition of L0 backend should decide whether it also will require such attributes at some point, and then whether SYCL (and in turn UR) will want to support them. 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 am fine with it being integer, but ask this to be documented (comments in pi.h) 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. Done 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. OK I forgot the to document the return type. I will do this now. Note that this is a comment so won't affect the test status of this PR that is all green (everything passing.) |
||
|
||
using pi_mem_info = _pi_mem_info; | ||
using pi_peer_attr = _pi_peer_attr; | ||
|
||
// | ||
// Following section contains SYCL RT Plugin Interface (PI) functions. | ||
|
@@ -1087,6 +1099,14 @@ __SYCL_EXPORT pi_result piDevicesGet(pi_platform platform, | |
pi_uint32 num_entries, pi_device *devices, | ||
pi_uint32 *num_devices); | ||
|
||
__SYCL_EXPORT pi_result piextEnablePeerAccess(pi_device command_device, | ||
pi_device peer_device); | ||
__SYCL_EXPORT pi_result piextDisablePeerAccess(pi_device command_device, | ||
pi_device peer_device); | ||
__SYCL_EXPORT pi_result piextPeerAccessGetInfo( | ||
pi_device command_device, pi_device peer_device, pi_peer_attr attr, | ||
size_t param_value_size, void *param_value, size_t *param_value_size_ret); | ||
|
||
steffenlarsen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// Returns requested info for provided native device | ||
/// Return PI_DEVICE_INFO_EXTENSION_DEVICELIB_ASSERT for | ||
/// PI_DEVICE_INFO_EXTENSIONS query when the device supports native asserts | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make the following changes to the API specification:
Update the "Status" section using the wording in the template.
Add a section "Backend support status" noting that this extension is supported only for the CUDA backend. I'd suggest wording like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion, I've made these changes now.