[SYCL] Implement register_host_memory extension API#22390
Open
againull wants to merge 9 commits into
Open
Conversation
Replace the placeholder TBD flag in the host memory registration flags enum with UR_EXP_USM_HOST_ALLOC_REGISTER_FLAG_READ_ONLY and regenerate the API headers. The flag indicates that device access to the registered range is read-only. In the Level Zero v2 adapter, map the flag onto ZE_HOST_MEM_ALLOC_FLAG_MEM_READ_ONLY when calling zeMemAllocHost so the driver registers the external system memory range in read-only device-access mode. Assisted-By: Claude
- Add the sycl::ext::oneapi::experimental::register_host_memory and unregister_host_memory free functions defined by sycl_ext_oneapi_register_host_memory. - Implement read_only property. - Add a unit tests to verify argument forwarding to the UR host memory registration APIs. - Add e2e test that registers a page-aligned host allocation and checks that the pointer can be used in a kernel, that explicit copies to and from it work etc. Assisted-By: Claude
againull
force-pushed
the
reg_mem_sycl_rt
branch
from
9a76ede to
f89d360
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces the experimental sycl_ext_oneapi_register_host_memory extension into the SYCL runtime and headers, adding the public API surface plus device/aspect plumbing and accompanying unit/e2e tests.
Changes:
- Add
sycl::ext::oneapi::experimental::register_host_memory/unregister_host_memoryAPIs andread_onlyproperty lowering to UR registration flags. - Plumb the new device aspect (
ext_oneapi_register_host_memory) through SYCL aspect tables and UR device info querying. - Add unit + end-to-end tests and update ABI symbol dumps / feature-test macro.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| sycl/include/sycl/ext/oneapi/experimental/register_host_memory.hpp | New public extension header and read_only property key. |
| sycl/source/detail/usm/usm_impl.cpp | Runtime implementation calling UR host alloc register/unregister APIs. |
| sycl/include/sycl/info/aspects.def | Add new aspect id for host memory registration support. |
| sycl/source/detail/device_impl.hpp | Report the new aspect via UR device info. |
| sycl/source/detail/ur_device_info_ret_types.inc | Add return type mapping for new UR device info query. |
| sycl/include/sycl/ext/oneapi/properties/property.hpp | Add new compile-time property kind for read_only. |
| sycl/source/feature_test.hpp.in | Add SYCL_EXT_ONEAPI_REGISTER_HOST_MEMORY feature-test macro. |
| sycl/include/sycl/sycl.hpp | Export the new extension header via the umbrella include. |
| sycl/unittests/Extensions/RegisterHostMemory.cpp | New unit tests using UR mock to validate behavior/forwarding. |
| sycl/unittests/Extensions/CMakeLists.txt | Hook the new unit test into the ExtensionsTests target. |
| sycl/test-e2e/USM/register_host_memory.cpp | New end-to-end functional test for registration semantics. |
| sycl/test/abi/sycl_symbols_linux.dump | ABI update for newly exported runtime entry points. |
| sycl/test/abi/sycl_symbols_windows.dump | ABI update for newly exported runtime entry points. |
| llvm/include/llvm/SYCLLowerIR/DeviceConfigFile.td | Add aspect to SYCLLowerIR aspect lists (test config). |
zxue2
reviewed
Jun 24, 2026
| TEST_F(RegisterHostMemoryTests, RegisterAndUnregisterForwardArguments) { | ||
| int Storage = 0; | ||
| void *Ptr = &Storage; | ||
| constexpr size_t Size = 4096; |
There was a problem hiding this comment.
Do you mind adding hugepage(e.g., 2M) test case? thx
Contributor
Author
Contributor
Author
|
I've addressed copilot comments. @uditagarwal97 @intel/dpcpp-tools-reviewers Could you please take a look. |
Comment on lines
+672
to
+674
| if (NumBytes == 0) | ||
| throw sycl::exception(make_error_code(errc::invalid), | ||
| "register_host_memory: size must not be zero."); |
Contributor
There was a problem hiding this comment.
I'm just rethinking if it's correct to throw when NumBytes is zero. Do we also throw if number of bytes are zero in sycl::malloc_host? At least, C's malloc does not throw and just return NULL or a valid pointer that may not be dereferenced. I wonder if we should also do the same?
Comment on lines
+625
to
+633
| static size_t getHostPageSize() { | ||
| #ifdef _WIN32 | ||
| SYSTEM_INFO Info; | ||
| GetSystemInfo(&Info); | ||
| return static_cast<size_t>(Info.dwPageSize); | ||
| #else | ||
| return static_cast<size_t>(sysconf(_SC_PAGESIZE)); | ||
| #endif | ||
| } |
Contributor
There was a problem hiding this comment.
Can we cache the value of page size, so that subsequent calls to getHostPageSize() are faster?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sycl_ext_oneapi_register_host_memory.
Related extension specification: #22324
Assisted-By: Claude