Skip to content

Commit d0b4788

Browse files
lslusarczykldorau
authored andcommitted
[UR][L0] Memory resident limit to enabled peers
Previously, USM memory was made resident on all peer devices regardless of whether peer access had been explicitly enabled by the user. This change limits memory residency to only those peer devices for which peer access has been enabled via urUsmP2PEnablePeerAccessExp. To support this, a per-device `peers` vector is introduced (ur_device_handle_t_::PeerStatus) that tracks the peer-access state (ENABLED, DISABLED, NO_CONNECTION) for every device on the platform. The vector is populated at platform initialisation time using zeDeviceGetP2PProperties and updated atomically when urUsmP2PEnablePeerAccessExp / urUsmP2PDisablePeerAccessExp are called. Key changes: - platform.cpp: populate `peers` for every device during device-cache initialisation. - v2/context.cpp: add changeResidentDevice(), getDevicesWhoseAllocationsCanBeAccessedFrom(), and getDevicesWhichCanAccessAllocationsPresentOn() to update memory residency in existing USM pools when peer-access state changes. Fix urContextRelease to only remove the context from Platform->Contexts and delete it when the reference count actually reaches zero, preventing live contexts from being missed during subsequent peer-access updates. - v2/usm_p2p.cpp: new file implementing urUsmP2PEnablePeerAccessExp, urUsmP2PDisablePeerAccessExp, and urUsmP2PPeerAccessGetInfoExp for the v2 adapter. Fix a data race where peers[] was written under a shared (read) lock; changed to an exclusive scoped_lock. - v2/usm.cpp: pass the list of enabled peer devices to the USM pool/provider so that zeMemMakeResident is called only for those devices. - sycl/source/device.cpp: add a cross-platform check in ext_oneapi_enable_peer_access / ext_oneapi_disable_peer_access and throw sycl::exception(errc::invalid) when the two devices belong to different platforms. - ur_pool_manager.hpp: propagate resident-devices information through pool descriptors. Co-authored-by: Łukasz Ślusarczyk <lukasz.slusarczyk@intel.com> Co-authored-by: Lukasz Dorau <lukasz.dorau@intel.com> Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
1 parent 929fd90 commit d0b4788

18 files changed

Lines changed: 611 additions & 114 deletions

File tree

.github/copilot-instructions.md

Lines changed: 164 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,164 @@
1-
When performing a code review, pay close attention to code modifying a function's
2-
control flow. Could the change result in the corruption of performance profile
3-
data? Could the change result in invalid debug information, in particular for
4-
branches and calls?
1+
# Intel LLVM (DPC++ / oneAPI) — Copilot Instructions
2+
3+
## Repository overview
4+
5+
This is Intel's LLVM fork (`intel/llvm`, `sycl` branch), hosting:
6+
- **oneAPI DPC++ compiler** — a SYCL implementation built on top of Clang/LLVM
7+
- **DPC++ runtime library** (`sycl/`)
8+
- **Unified Runtime (UR)** (`unified-runtime/`) — backend abstraction over Level-Zero, OpenCL, CUDA, HIP
9+
- **LLVM-SPIRV translator** (`llvm-spirv/`)
10+
- **SYCL JIT** (`sycl-jit/`), instrumentation (`xpti/`, `xptifw/`), device libraries (`libdevice/`)
11+
12+
For non-SYCL/DPC++ changes, submit patches upstream to https://llvm.org/ directly.
13+
14+
## Build
15+
16+
Use the buildbot scripts (default output directory: `./build`):
17+
18+
```bash
19+
python buildbot/configure.py # Run CMake; use --help for full flag list
20+
python buildbot/compile.py # Build; default target is deploy-sycl-toolchain
21+
```
22+
23+
Common `configure.py` flags: `--cuda`, `--hip`, `--native_cpu`, `--shared-libs`, `-t Debug`, `-o <build_dir>`, `--werror`
24+
Common `compile.py` flags: `-o <build_dir>`, `-t <target>` (e.g. `clang`, `llvm-spirv`), `-j <N>`
25+
26+
After build, add to environment:
27+
```bash
28+
export PATH=$DPCPP_HOME/llvm/build/bin:$PATH
29+
export LD_LIBRARY_PATH=$DPCPP_HOME/llvm/build/lib:$LD_LIBRARY_PATH
30+
```
31+
32+
## Testing
33+
34+
### In-tree LIT tests (device-independent)
35+
36+
Run the full check via the buildbot helper:
37+
```bash
38+
python buildbot/check.py
39+
```
40+
41+
Or run individual check targets from the build directory:
42+
```bash
43+
ninja check-sycl # DPC++ headers + runtime unit tests
44+
ninja check-clang # Clang (including SYCL FE tests)
45+
ninja check-llvm # Core LLVM
46+
ninja check-llvm-spirv # LLVM-SPIRV translator
47+
```
48+
49+
Run a single LIT test file:
50+
```bash
51+
llvm-lit sycl/test/path/to/test.cpp
52+
llvm-lit clang/test/path/to/test.cpp
53+
```
54+
55+
### E2E tests (require hardware / backend runtimes)
56+
57+
```bash
58+
# Run all E2E tests on all available devices
59+
llvm-lit sycl/test-e2e
60+
61+
# Limit to specific devices
62+
llvm-lit --param sycl_devices="level_zero:gpu;opencl:cpu" sycl/test-e2e
63+
64+
# Run a single E2E test
65+
llvm-lit sycl/test-e2e/Basic/vector_add.cpp
66+
```
67+
68+
### Unified Runtime tests (from UR build directory)
69+
70+
```bash
71+
make check-unified-runtime # All UR tests
72+
python -m lit test/conformance/device/ -a --filter="urDeviceGetInfo" # Single UR conformance test
73+
```
74+
75+
## Code formatting
76+
77+
Use `git-clang-format` against the `sycl` branch merge base. Match the CI `clang-format` version (see `.github/workflows/pr-code-format.yml`):
78+
79+
```bash
80+
# Check format of staged changes
81+
./clang/tools/clang-format/git-clang-format `git merge-base origin/sycl HEAD`
82+
83+
# Also fix unstaged changes
84+
./clang/tools/clang-format/git-clang-format -f `git merge-base origin/sycl HEAD`
85+
```
86+
87+
## Architecture
88+
89+
### DPC++ compilation pipeline
90+
91+
1. **Clang front-end** (`clang/`) — parses SYCL source, outlines device code, generates integration headers, enforces device-code restrictions (no exceptions, no virtual calls)
92+
2. **Device middle-end / back-end** — applies LLVM passes (address space inference, etc.), translates to SPIR-V via `llvm-spirv`, or targets NVPTX/AMDGCPU directly
93+
3. **Host compilation** — standard Clang compilation of host code
94+
4. **Linking**`clang-offload-wrapper` embeds device images into the fat binary
95+
5. **Runtime** (`sycl/source/`) — manages command queues, buffers, kernels; dispatches work to backend via Unified Runtime
96+
6. **Unified Runtime** (`unified-runtime/`) — adapters translate UR calls to Level-Zero, OpenCL, CUDA, or HIP
97+
98+
### Key directories
99+
100+
| Path | Contents |
101+
|---|---|
102+
| `clang/lib/Sema/`, `clang/lib/CodeGen/` | SYCL-specific FE logic |
103+
| `sycl/include/sycl/` | Public SYCL headers |
104+
| `sycl/source/` | DPC++ runtime library implementation |
105+
| `sycl/test/` | Device-independent LIT + unit tests |
106+
| `sycl/test-e2e/` | End-to-end tests (need real hardware) |
107+
| `sycl/unittests/` | GoogleTest unit tests (use UrMock, no device required) |
108+
| `unified-runtime/source/adapters/` | Level-Zero, OpenCL, CUDA, HIP adapters |
109+
| `unified-runtime/scripts/core/*.yml` | UR API spec (source of truth — run `make generate` after edits) |
110+
| `llvm-spirv/` | LLVM IR ↔ SPIR-V translator |
111+
| `sycl-jit/` | JIT/runtime compilation support |
112+
| `xpti/`, `xptifw/` | Instrumentation (XPTI tracing) |
113+
114+
### Unified Runtime code generation
115+
116+
UR API is defined in YAML (`unified-runtime/scripts/core/*.yml`). After any YAML changes:
117+
```bash
118+
# From unified-runtime build directory
119+
make generate
120+
```
121+
Do **not** hand-edit generated files (`ur_api.h`, `ur_ddi.h`, `ur_libapi.cpp`, `ur_ldrddi.cpp`, `ur_valddi.cpp`, etc.).
122+
123+
## Key conventions
124+
125+
### Commit messages & PRs
126+
127+
- SYCL/DPC++ changes must include `[SYCL]` in the title. Add sub-tags for components: `[UR]`, `[CUDA]`, `[HIP]`, `[Doc]`, `[NFC]`.
128+
- Commits are squash-merged; the PR description becomes the commit message — keep it accurate.
129+
- Gatekeepers: `@intel/llvm-gatekeepers`.
130+
131+
### ABI/API stability
132+
133+
- Public DPC++ APIs must remain ABI/API compatible. Experimental APIs are exempted (placed in experimental namespaces).
134+
- Deprecation order: implement new API → add deprecation warning → wait → remove from headers → remove from library.
135+
- See `sycl/doc/developer/ABIPolicyGuide.md`.
136+
137+
### Test placement
138+
139+
| Test type | Location | When to use |
140+
|---|---|---|
141+
| Clang FE / compile-time checks | `clang/test/ASTSYCL/`, `sycl/test/` | Checking diagnostics, IR, `static_assert` |
142+
| Runtime unit tests (no device) | `sycl/unittests/` | Runtime behavior, use `UrMock` |
143+
| Device-independent LIT | `sycl/test/` | Header/IR checks, syntax-only |
144+
| E2E (requires backend) | `sycl/test-e2e/` | Full stack, actual device execution |
145+
146+
- Use `-fsyntax-only` when only checking compilation succeeds.
147+
- Device code checks go under `sycl/test/check_device_code/`.
148+
- Use `sycl::` namespace, not `cl::sycl::`.
149+
- Always redirect compiler output to `%t` (temp) to avoid race conditions in parallel lit runs.
150+
151+
### SYCL-specific C++ rules
152+
153+
- Never `#include <iostream>` in library/header files. Use `sycl/detail/iostream_proxy.hpp` instead (avoids static constructors).
154+
- All identifiers in `llvm/sycl` headers must contain at least one lowercase letter (avoids macro conflicts).
155+
- Target C++17.
156+
157+
### Release notes
158+
159+
Record changes in `sycl/ReleaseNotes.md` under "upcoming release" when changing public interfaces, fixing bugs, or making observable behavioral changes. Reference PRs as `intel/llvm#NNNNN`.
160+
161+
### Code review focus
162+
163+
- When reviewing changes to control flow, check whether the change could corrupt performance profile data or produce invalid debug information (especially for branches and calls).
164+
- For LLDB code specifically, see `.github/instructions/lldb.instructions.md` for naming conventions (`snake_case` variables, `UpperCamelCase` methods, `m_`/`s_`/`g_` prefixes) and other LLDB-specific rules.

sycl/source/device.cpp

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -212,19 +212,33 @@ bool device::has(aspect Aspect) const { return impl->has(Aspect); }
212212
void device::ext_oneapi_enable_peer_access(const device &peer) {
213213
ur_device_handle_t Device = impl->getHandleRef();
214214
ur_device_handle_t Peer = peer.impl->getHandleRef();
215-
if (Device != Peer) {
216-
detail::adapter_impl &Adapter = impl->getAdapter();
217-
Adapter.call<detail::UrApiKind::urUsmP2PEnablePeerAccessExp>(Device, Peer);
215+
216+
if (Device == Peer)
217+
return;
218+
219+
if (peer.get_platform() != get_platform()) {
220+
throw exception(errc::invalid,
221+
"Can not enable peer access between different platforms");
218222
}
223+
224+
impl->getAdapter().call<detail::UrApiKind::urUsmP2PEnablePeerAccessExp>(
225+
Device, Peer);
219226
}
220227

221228
void device::ext_oneapi_disable_peer_access(const device &peer) {
222229
ur_device_handle_t Device = impl->getHandleRef();
223230
ur_device_handle_t Peer = peer.impl->getHandleRef();
224-
if (Device != Peer) {
225-
detail::adapter_impl &Adapter = impl->getAdapter();
226-
Adapter.call<detail::UrApiKind::urUsmP2PDisablePeerAccessExp>(Device, Peer);
231+
232+
if (Device == Peer)
233+
return;
234+
235+
if (peer.get_platform() != get_platform()) {
236+
throw exception(errc::invalid,
237+
"Can not disable peer access between different platforms");
227238
}
239+
240+
impl->getAdapter().call<detail::UrApiKind::urUsmP2PDisablePeerAccessExp>(
241+
Device, Peer);
228242
}
229243

230244
bool device::ext_oneapi_can_access_peer(const device &peer,

unified-runtime/source/adapters/level_zero/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,6 @@ if(UR_BUILD_ADAPTER_L0_V2)
150150
${CMAKE_CURRENT_SOURCE_DIR}/helpers/kernel_helpers.cpp
151151
${CMAKE_CURRENT_SOURCE_DIR}/helpers/memory_helpers.cpp
152152
${CMAKE_CURRENT_SOURCE_DIR}/helpers/mutable_helpers.cpp
153-
${CMAKE_CURRENT_SOURCE_DIR}/usm_p2p.cpp
154153
${CMAKE_CURRENT_SOURCE_DIR}/virtual_mem.cpp
155154
${CMAKE_CURRENT_SOURCE_DIR}/../../ur/ur.cpp
156155
${CMAKE_CURRENT_SOURCE_DIR}/sampler.hpp
@@ -194,6 +193,7 @@ if(UR_BUILD_ADAPTER_L0_V2)
194193
${CMAKE_CURRENT_SOURCE_DIR}/v2/queue_immediate_in_order.cpp
195194
${CMAKE_CURRENT_SOURCE_DIR}/v2/queue_immediate_out_of_order.cpp
196195
${CMAKE_CURRENT_SOURCE_DIR}/v2/usm.cpp
196+
${CMAKE_CURRENT_SOURCE_DIR}/v2/usm_p2p.cpp
197197
)
198198
install_ur_library(ur_adapter_level_zero_v2)
199199

unified-runtime/source/adapters/level_zero/context.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ ur_result_t urContextCreate(
4141

4242
Context->initialize();
4343
*RetContext = reinterpret_cast<ur_context_handle_t>(Context);
44+
// TODO: delete below 'if' when memory isolation in the context is
45+
// implemented in the driver
4446
if (IndirectAccessTrackingEnabled) {
4547
std::scoped_lock<ur_shared_mutex> Lock(Platform->ContextsMutex);
4648
Platform->Contexts.push_back(*RetContext);

unified-runtime/source/adapters/level_zero/device.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2353,3 +2353,24 @@ void ZeUSMImportExtension::doZeUSMRelease(ze_driver_handle_t DriverHandle,
23532353
void *HostPtr) {
23542354
ZE_CALL_NOCHECK(zexDriverReleaseImportedPointer, (DriverHandle, HostPtr));
23552355
}
2356+
2357+
std::ostream &operator<<(std::ostream &os,
2358+
ur_device_handle_t_ const &device_handle) {
2359+
if (device_handle.Id.has_value()) {
2360+
return os << device_handle.Id.value();
2361+
}
2362+
return os << "NONE";
2363+
}
2364+
2365+
std::ostream &operator<<(std::ostream &os,
2366+
ur_device_handle_t_::PeerStatus peer_status) {
2367+
switch (peer_status) {
2368+
case ur_device_handle_t_::PeerStatus::DISABLED:
2369+
return os << "DISABLED";
2370+
case ur_device_handle_t_::PeerStatus::ENABLED:
2371+
return os << "ENABLED";
2372+
case ur_device_handle_t_::PeerStatus::NO_CONNECTION:
2373+
return os << "NO_CONNECTION";
2374+
}
2375+
return os << "UNKNOWN";
2376+
}

unified-runtime/source/adapters/level_zero/device.hpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,17 +261,29 @@ struct ur_device_handle_t_ : ur_object {
261261
std::unordered_map<ur_exp_image_native_handle_t, ze_image_handle_t>
262262
ZeOffsetToImageHandleMap;
263263

264+
// Devices which user enabled p2p access by
265+
// urUsmP2P(Enable|Disable)PeerAccessExp. Devices are indexed by device id.
266+
enum class PeerStatus : char { ENABLED, DISABLED, NO_CONNECTION };
267+
std::vector<PeerStatus>
268+
peers; // info if our device can access given peer device allocations
269+
264270
// unique ephemeral identifer of the device in the adapter
265271
std::optional<DeviceId> Id;
266272

267273
ur::RefCount RefCount;
268274
};
269275

276+
std::ostream &operator<<(std::ostream &os,
277+
ur_device_handle_t_ const &device_handle);
278+
std::ostream &operator<<(std::ostream &os,
279+
ur_device_handle_t_::PeerStatus peer_status);
280+
270281
// Collects a flat vector of unique devices for USM memory pool creation.
271282
// Traverses the input devices and their sub-devices, ensuring each Level Zero
272283
// device handle appears only once in the result.
273284
inline std::vector<ur_device_handle_t> CollectDevicesForUsmPoolCreation(
274285
const std::vector<ur_device_handle_t> &Devices) {
286+
275287
std::vector<ur_device_handle_t> DevicesAndSubDevices;
276288
std::unordered_set<ze_device_handle_t> Seen;
277289

unified-runtime/source/adapters/level_zero/platform.cpp

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -746,9 +746,9 @@ ur_platform_handle_t_::getDeviceFromNativeHandle(ze_device_handle_t ZeDevice) {
746746
std::shared_lock<ur_shared_mutex> Lock(URDevicesCacheMutex);
747747
auto it = std::find_if(URDevicesCache.begin(), URDevicesCache.end(),
748748
[&](std::unique_ptr<ur_device_handle_t_> &D) {
749-
return D.get()->ZeDevice == ZeDevice &&
750-
(D.get()->RootDevice == nullptr ||
751-
D.get()->RootDevice->RootDevice == nullptr);
749+
return D->ZeDevice == ZeDevice &&
750+
(D->RootDevice == nullptr ||
751+
D->RootDevice->RootDevice == nullptr);
752752
});
753753
if (it != URDevicesCache.end()) {
754754
return (*it).get();
@@ -914,6 +914,44 @@ ur_result_t ur_platform_handle_t_::populateDeviceCacheIfNeeded() {
914914
ZeDeviceSynchronizeSupported = Supported;
915915
}
916916

917+
for (auto &dev : URDevicesCache) {
918+
dev->peers = std::vector<ur_device_handle_t_::PeerStatus>(
919+
URDevicesCache.size(), ur_device_handle_t_::PeerStatus::NO_CONNECTION);
920+
921+
for (size_t peerId = 0; peerId < URDevicesCache.size(); ++peerId) {
922+
if (peerId == dev->Id.value())
923+
continue;
924+
925+
ZeStruct<ze_device_p2p_properties_t> p2pProperties;
926+
ZE2UR_CALL_THROWS(
927+
zeDeviceGetP2PProperties,
928+
(dev->ZeDevice, URDevicesCache[peerId]->ZeDevice, &p2pProperties));
929+
if (!(p2pProperties.flags & ZE_DEVICE_P2P_PROPERTY_FLAG_ACCESS)) {
930+
UR_LOG(INFO,
931+
"p2p access to memory of dev:{} from dev:{} not possible due to "
932+
"lack of p2p property",
933+
peerId, dev->Id.value());
934+
continue;
935+
}
936+
937+
ze_bool_t p2p;
938+
ZE2UR_CALL_THROWS(
939+
zeDeviceCanAccessPeer,
940+
(dev->ZeDevice, URDevicesCache[peerId]->ZeDevice, &p2p));
941+
if (!p2p) {
942+
UR_LOG(INFO,
943+
"p2p access to memory of dev:{} from dev:{} not possible due to "
944+
"no connection",
945+
peerId, dev->Id.value());
946+
continue;
947+
}
948+
949+
UR_LOG(INFO, "p2p access to memory of dev:{} from dev:{} can be enabled",
950+
peerId, dev->Id.value());
951+
dev->peers[peerId] = ur_device_handle_t_::PeerStatus::DISABLED;
952+
}
953+
}
954+
917955
return UR_RESULT_SUCCESS;
918956
}
919957

unified-runtime/source/adapters/level_zero/platform.hpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,10 @@ struct ur_platform_handle_t_ : ur::handle_base<ur::level_zero::ddi_getter>,
9797
uint32_t VersionMinor,
9898
uint32_t VersionBuild);
9999

100-
// Keep track of all contexts in the platform. This is needed to manage
101-
// a lifetime of memory allocations in each context when there are kernels
102-
// with indirect access.
103-
// TODO: should be deleted when memory isolation in the context is implemented
104-
// in the driver.
100+
// Keep track of all contexts in the platform. In v1 L0 this is needed to
101+
// manage a lifetime of memory allocations in each context when there are
102+
// kernels with indirect access. In v2 it is used during
103+
// ext_oneapi_enable_peer_access and ext_oneapi_disable_peer_access calls.
105104
std::list<ur_context_handle_t> Contexts;
106105
ur_shared_mutex ContextsMutex;
107106

unified-runtime/source/adapters/level_zero/usm_p2p.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,23 @@
1212

1313
namespace ur::level_zero {
1414

15-
ur_result_t urUsmP2PEnablePeerAccessExp(ur_device_handle_t /*commandDevice*/,
16-
ur_device_handle_t /*peerDevice*/) {
15+
ur_result_t urUsmP2PEnablePeerAccessExp(ur_device_handle_t commandDevice,
16+
ur_device_handle_t peerDevice) {
1717

18-
// L0 has peer devices enabled by default
18+
UR_LOG(INFO,
19+
"user enables peer access to memory of {} from {}, ignored, in V1 P2P "
20+
"is always enabled",
21+
*commandDevice, *peerDevice);
1922
return UR_RESULT_SUCCESS;
2023
}
2124

22-
ur_result_t urUsmP2PDisablePeerAccessExp(ur_device_handle_t /*commandDevice*/,
23-
ur_device_handle_t /*peerDevice*/) {
25+
ur_result_t urUsmP2PDisablePeerAccessExp(ur_device_handle_t commandDevice,
26+
ur_device_handle_t peerDevice) {
2427

25-
// L0 has peer devices enabled by default
28+
UR_LOG(INFO,
29+
"user disables peer access to memory of {} from {}, ignored, in V1 "
30+
"P2P is always enabled",
31+
*commandDevice, *peerDevice);
2632
return UR_RESULT_SUCCESS;
2733
}
2834

0 commit comments

Comments
 (0)