[SYCL] Add RT dependency on interface layer for offloading #4

KseniyaTikhomirova · 2025-07-11T11:26:11Z

This is part of the SYCL support upstreaming effort. The relevant RFCs can
be found here:

https://discourse.llvm.org/t/rfc-add-full-support-for-the-sycl-programming-model/74080
https://discourse.llvm.org/t/rfc-sycl-runtime-upstreaming/74479

The SYCL runtime is device-agnostic and uses Unified Runtime (GitHub -
oneapi-src/unified-runtime) as an external dependency. This Unified Runtime
serves as an interface layer between the SYCL runtime and device-specific
backends. Unified Runtime has several adapters that bind to various backends.

NOTE: UR is considered as temporal solution until llvm-project/offload is
fully functional and is able to replace UR.

This commit adds:

fetching UR, UR build as dependency, document with a short overview of UR
with links to repos and documentation.

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova · 2025-07-23T16:36:04Z

This PR:

adds minimal linux build workflow (I reused libcxx container for now)
adds UR fetching, libsycl cmake files don't have any install commands, everything seems to be done already by UMF & UR & adapter targets handling in UR repo.
native_cpu and level_zero_v2 adapters are not mentioned/included. I think it is better to upload bare minimum now. Other adapters are easy to include later, once we'll get approval for UR addition from community.
LLVM_ENABLE_PIC check and -fPIC usage are changed in this PR since I build libsycl alone in workflow steps and have to enable fPIC explicitly (previously it was done by top project settings, see LLVM_ENABLE_PIC).
I haven't found any LLVM requirements/limitations about installation rules. I am not confident about include content with extra targets like UMF/UR. As you can see in Compile output in "Build libsycl" include folder is populated with new headers and top level content looks like:

$ ls ./install/release/include/
CL  level_zero  sycl  umf  umf.h  ur_api_funcs.def  ur_api.h  ur_ddi.h  ur_print.h  ur_print.hpp

FYI @tahonermann, @dvrogozh, @asudarsa, @aelovikov-intel, @sergey-semenov

dvrogozh

I still strongly believe that adding external dependency on UR even as a temporary solution to later switch to liboffload is a wrong step to do. This will tight effort to the external project (UR) which potentially supports much more cases than initially will be implemented in liboffload. That might lead to problems switching to libofffload and might artifically delay the switch. I still suggest that final solutions needs to be worked on instead. This seems more pragmatic from my perspective.

In case you want to proceed with UR approach I suggest to add a description to the readme on UR in this PR to highlight liboffload considerations and temporal status of UR solution with the instructions on how developers can work on liboffload for sycl.

dvrogozh · 2025-07-23T17:32:31Z

libsycl/docs/DesignDocs/UnifiedRuntime.rst

+library, and the adapter libraries that implement the API for various backends.
+
+The SYCL runtime accesses the UR API via the Adapter object. Each Adapter
+object owns a ur_adapter_handle_t, which represents a UR backend (e.g. OpenCL,


Suggested change

object owns a ur_adapter_handle_t, which represents a UR backend (e.g. OpenCL,

object owns a ``ur_adapter_handle_t``, which represents a UR backend (e.g. OpenCL,

done in fe6b582

dvrogozh · 2025-07-23T17:34:14Z

libsycl/docs/DesignDocs/UnifiedRuntime.rst

+
+The Unified Runtime (UR) project serves as an interface layer between the SYCL
+runtime and the device-specific runtime layers which control execution on
+devices. The parts of it primarily utilized by SYCL RT are its C API, loader


"The parts of it primarily utilized by SYCL RT are its C API, loader
library, and the adapter libraries that implement the API for various backends."

This one reads too heavy. Can we reformulate this simpler? Maybe starting with "SYCL RT utilizes its C API, ...."?

done in fe6b582

dvrogozh · 2025-07-23T17:36:09Z

libsycl/docs/DesignDocs/UnifiedRuntime.rst

+   :local:
+
+.. _unified runtime:
+


It was highlighted in the discussion of #2 that UR is a temporary solution. Can this be described here in this document noting what the permanent solution will be and how to try it out (and work on it) instead?

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova · 2025-08-01T12:36:37Z

I still strongly believe that adding external dependency on UR even as a temporary solution to later switch to liboffload is a wrong step to do. This will tight effort to the external project (UR) which potentially supports much more cases than initially will be implemented in liboffload. That might lead to problems switching to libofffload and might artifically delay the switch. I still suggest that final solutions needs to be worked on instead. This seems more pragmatic from my perspective.

In case you want to proceed with UR approach I suggest to add a description to the readme on UR in this PR to highlight liboffload considerations and temporal status of UR solution with the instructions on how developers can work on liboffload for sycl.

I do agree that it is preferrable to work with liboffload right from the beginning. Although liboffload now is not stable even from architectural and API point of view. Tagging @stdale-intel for more info. Waiting for liboffload readiness will block our effort and effort of clang upstreaming as well since SYCL implementation involves many components that have to work together.
"This will tight effort to the external project (UR) which potentially supports much more cases than initially will be implemented in liboffload" That is not the case. SYCL RT upstreaming is in the very beginning and we will be focused on basic functionality first. Basic functionality that must be present in liboffload at some point otherwise there is no sense to use liboffload at all.

I don't agree that I should document UR & liboffload replacements in UR related documentation. I will add notes into index.rst that now contains some details about current status of upstreaming & plans. Anyway I am not hiding that UR is a temporal solution - this data is present in PR description. But I can't provide any instructions about how people can work on liboffload for sycl while that project is under discussion and AFAIK doesn't even have stable architecture and API. @stdale-intel please correct me if I misunderstood smth.

KseniyaTikhomirova · 2025-08-05T08:41:58Z

Hi @tahonermann, @dvrogozh, @asudarsa, @aelovikov-intel, @sergey-semenov,
do you have any/more comments regarding this change? Should I publish it as it is then?

aelovikov-intel · 2025-08-05T17:29:17Z

.github/workflows/libsycl-build-and-test.yaml

+    runs-on: ubuntu-22.04
+    # reuse libcxx container for now
+    container: ghcr.io/llvm/libcxx-linux-builder:2b57ebb50b6d418e70382e655feaa619b558e254
+    continue-on-error: false


Isn't that the default?

I haven't seen this explicitly stated anywhere. https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#jobsjob_idcontinue-on-error
it sounds like it should be the default but nothing bad will happen if I explicitly specify my intention here.

aelovikov-intel · 2025-08-05T17:32:29Z

libsycl/cmake/Modules/FetchUnifiedRuntime.cmake

+set(LIBSYCL_UR_OVERRIDE_FETCH_CONTENT_REPO
+  "" CACHE STRING "Override the Unified Runtime FetchContent repository")
+set(LIBSYCL_UR_OVERRIDE_FETCH_CONTENT_TAG
+  "" CACHE STRING "Override the Unified Runtime FetchContent tag")
+
+# Options to disable use of FetchContent to include Unified Runtime source code
+# to improve developer workflow.
+option(LIBSYCL_UR_USE_FETCH_CONTENT
+  "Use FetchContent to acquire the Unified Runtime source code" ON)
+set(LIBSYCL_UR_SOURCE_DIR
+  "" CACHE PATH "Path to root of Unified Runtime repository")


@sarnex, I think you're our CMake expert, can you take a look at the CMake part in this PR, please?

I think you're our CMake expert

Oh god please no

aelovikov-intel · 2025-08-05T17:38:16Z

.github/workflows/libsycl-build-and-test.yaml

+      - name: Register cleanup after job is finished
+        uses: ./libsycl/utils/ci/actions/cleanup


I don't think we need any of that here. AFAIK, cleanup is necessary in our syclos CI because we use self-hosted runners and the working directory isn't cleaned up between tasks. With GH-provided runner that isn't an issue.

I requested system for self-hosted runner. So I do expect us to need this in future. Although you are right about current state, I will remove it then.

done in 918ba92

aelovikov-intel · 2025-08-05T17:38:40Z

libsycl/utils/ci/actions/cleanup/action.yml

@@ -0,0 +1,7 @@
+name: 'Cleanup'


All three files here can be dropped, IMO, see my earlier comment.

done in 918ba92

sarnex · 2025-08-05T17:46:56Z

libsycl/cmake/Modules/FetchUnifiedRuntime.cmake

+
+# Options to override the default behaviour of the FetchContent to include UR
+# source code.
+set(LIBSYCL_UR_OVERRIDE_FETCH_CONTENT_REPO


In the current pattern we aren't having any variables for repo override, tag override, or to use FetchContent. The only way a user can specify anything is by overriding the FetchContent automatically generated variable which is FETCHCONTENT_SOURCE_DIR_<uppercaseName> which will tell CMake to use that directory instead of downloading anything, we don't need to do anything to handle it.

So right now I've been doing:

if(NOT FETCHCONTENT_SOURCE_DIR_(converttouppsersomehow(${fetch-name}))
Ideally find_package, if not find_path
else()
FetchContent_Declare
...
...
endif()

Basically, assuming the user didn't explicitly specify a custom directory to use, search the system and if it's found use that, otherwise fall back to FetchContent which will either actually git clone or will use the user specified directory.

Example PR here

@sarnex what do you mean under "we don't have any variables for repo override... The only way a user can specify anything is by overriding..."?
we can easily override all present variables with:
for example -DLIBSYCL_UR_OVERRIDE_FETCH_CONTENT_REPO=something in the build command line.

Sorry, I mean 'the standard way it is being done for other dependencies is ...', so I was trying to say all other dependenices don't have user-settable variables for the repo url/tag but this one does, so I recommend removing them to simplify and align with how we handle other deps. Thanks

done in 9daa5a7
I also removed fetch_adapter_source since it brings extra detalization (comes from intel/llvm) which is unlikely to be used.

I also used find_package since UR has proper configs to use it.

music to my ears!

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

aelovikov-intel

I'm still throwing @sarnex under the bus carrying CMake expert status :D

sarnex

cmake lgtm minus nits! thanks!

sarnex · 2025-08-11T14:48:15Z

libsycl/cmake/Modules/FetchUnifiedRuntime.cmake

+if(NOT FETCHCONTENT_SOURCE_DIR_UNIFIED-RUNTIME)
+  find_package(unified-runtime)
+  if(unified-runtime_FOUND)
+    message ("STATUS: unified-runtime is found")


nit

Suggested change

message ("STATUS: unified-runtime is found")

message (STATUS "Found system install of unified-runtime")

or something like that

sarnex · 2025-08-11T14:51:51Z

libsycl/cmake/Modules/FetchUnifiedRuntime.cmake

+endif()
+
+# Disable errors from warnings while building the UR.
+# And remember origin flags before doing that.


nit

Suggested change

# And remember origin flags before doing that.

# And remember the original flags before doing that.

sarnex · 2025-08-11T14:52:27Z

libsycl/cmake/Modules/FetchUnifiedRuntime.cmake

+
+# Disable errors from warnings while building the UR.
+# And remember origin flags before doing that.
+set(CMAKE_CXX_FLAGS_BAK "${CMAKE_CXX_FLAGS}")


do we need to backup/restore CFLAGS as well?

libsycl will be warning free while UR build fails with Werror enabled. So yes, we can't use LLVM top level warning strategy for this dependency.

sorry i mean right now we are only backing up and restoring CMAKE_CXX_FLAGS, do we need to do CMAKE_C_FLAGS as well?

sarnex · 2025-08-11T14:53:33Z

libsycl/cmake/Modules/FetchUnifiedRuntime.cmake

+  "${unified-runtime_SOURCE_DIR}" CACHE PATH
+  "Path to Unified Runtime Headers" FORCE)
+
+set(UMF_BUILD_EXAMPLES OFF CACHE INTERNAL "EXAMPLES")


nit

Suggested change

set(UMF_BUILD_EXAMPLES OFF CACHE INTERNAL "EXAMPLES")

set(UMF_BUILD_EXAMPLES OFF CACHE INTERNAL "UMF EXAMPLES")

sarnex · 2025-08-11T14:54:19Z

I'm still throwing @sarnex under the bus carrying CMake expert status :D

I've brought this fate on myself

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova · 2025-08-11T15:32:25Z

I believe I've got feedback from RT side (Andrey), CMAKE expert (Nick), some comments from Arvind (in the former PR) and Dmitriy.
@tahonermann would you like to take a look? Or can I proceed then?

sarnex · 2025-08-11T15:34:09Z

CMAKE expert (Nick)

😬

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova added 3 commits July 11, 2025 04:24

Add UR dependency

cbd4127

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

build only sycl lib

b047308

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

remove extra cmake handling for ur

8866561

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova changed the title ~~Add UR dependency~~ [SYCL] Add RT dependency on interface layer for offloading Jul 23, 2025

KseniyaTikhomirova mentioned this pull request Jul 23, 2025

[SYCL] Add RT dependency on interface layer for offloading #2

Closed

KseniyaTikhomirova marked this pull request as ready for review July 23, 2025 16:21

dvrogozh reviewed Jul 23, 2025

View reviewed changes

fix comments

fe6b582

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova force-pushed the add_ur branch from 00d5ea4 to fe6b582 Compare August 1, 2025 12:22

aelovikov-intel reviewed Aug 5, 2025

View reviewed changes

sarnex reviewed Aug 5, 2025

View reviewed changes

KseniyaTikhomirova added 2 commits August 11, 2025 04:17

remove extra actions (cleanup)

918ba92

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

introduce usage of FETCHCONTENT_SOURCE_DIR_<name> for UR

9daa5a7

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova requested review from aelovikov-intel and sarnex August 11, 2025 14:45

aelovikov-intel approved these changes Aug 11, 2025

View reviewed changes

sarnex approved these changes Aug 11, 2025

View reviewed changes

fix nits

142c224

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova added 2 commits August 29, 2025 02:53

update UR tag

085f778

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

update UR again

c54afad

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

KseniyaTikhomirova force-pushed the add_ur branch from 67ec169 to c54afad Compare September 29, 2025 07:21

KseniyaTikhomirova closed this Oct 16, 2025

	object owns a ur_adapter_handle_t, which represents a UR backend (e.g. OpenCL,
	object owns a ``ur_adapter_handle_t``, which represents a UR backend (e.g. OpenCL,

		- name: Register cleanup after job is finished
		uses: ./libsycl/utils/ci/actions/cleanup

	message ("STATUS: unified-runtime is found")
	message (STATUS "Found system install of unified-runtime")

	# And remember origin flags before doing that.
	# And remember the original flags before doing that.

	set(UMF_BUILD_EXAMPLES OFF CACHE INTERNAL "EXAMPLES")
	set(UMF_BUILD_EXAMPLES OFF CACHE INTERNAL "UMF EXAMPLES")

[SYCL] Add RT dependency on interface layer for offloading #4

[SYCL] Add RT dependency on interface layer for offloading #4

Uh oh!

Conversation

KseniyaTikhomirova commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KseniyaTikhomirova commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dvrogozh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KseniyaTikhomirova commented Aug 1, 2025

Uh oh!

KseniyaTikhomirova commented Aug 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarnex Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarnex Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KseniyaTikhomirova Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aelovikov-intel left a comment

Choose a reason for hiding this comment

Uh oh!

sarnex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

KseniyaTikhomirova commented Jul 11, 2025 •

edited

Loading

KseniyaTikhomirova commented Jul 23, 2025 •

edited

Loading

sarnex Aug 5, 2025 •

edited

Loading

sarnex Aug 7, 2025 •

edited

Loading

KseniyaTikhomirova Aug 11, 2025 •

edited

Loading

sarnex Aug 11, 2025 •

edited

Loading