[SYCL] Add libsycl, a SYCL RT library implementation project #144372

KseniyaTikhomirova · 2025-06-16T15:38:10Z

This patch introduces libsycl, a SYCL runtime library implementation, as a top-level LLVM runtime project.
SYCL spec: https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html

Commit contains the basic folder layout and CMake infrastructure to build a dummy SYCL library.

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

Upcoming PRs:

UR offloading library fetch & build
partial implementation of sycl::platform: requires offloading layer, requires classes for backend loading & enumeration.

This patch introduces libsycl, a SYCL runtime library implementation, as a top-level LLVM runtime project. It contains the basic folder layout and CMake infrastructure to build a dummy SYCL library.

github-actions · 2025-06-16T15:38:28Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

KseniyaTikhomirova · 2025-06-16T15:41:33Z

FYI: @tahonermann @dvrogozh @asudarsa @aelovikov-intel

philnik777

Disclaimer: I haven't seen the RFCs related to this before and haven't read through them in detail.

I don't know much about SYCL, but I do know quite a bit about libc++ and why we do things in certain ways there. In case you're interested I'd be happy to talk to you about that. Feel free to contact me on Discord, Discourse or E-Mail if you'd like to set up a meeting.

philnik777 · 2025-06-16T15:55:08Z

libsycl/CMakeLists.txt

+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD_REQUIRED YES)
+set(CMAKE_CXX_EXTENSIONS OFF)


It's generally a good idea to do this on a per-target level instead of globally, since there are a lot of sub-projects with lots of different requirements.

you are right, I will remove cache entry here, thank you.

I interpreted @philnik777's suggestion as applying to all three of these settings, not just to the caching behavior of the first.

The original code is consistent with what is done for many projects. See flang-rt/CMakeLists.txt, lldb/CMakeLists.txt, openmp/CMakeLists.txt, llvm/CMakeLists.txt, lld/CMakeLists.txt, bolt/CMakeLists.txt, compiler-rt/CMakeLists.txt, mlir/CMakeLists.txt, clang/CMakeLists.txt, and offload/CMakeLists.txt. I'm not sure what the suggested alternative is. I guess changes to the add_sycl_rt_library function that defines the library targets?

Note that consumers of libsycl will require C++17 as well.

IMHO "The original code is consistent with what is done for many projects." is not correct.
Other projects has 2 build "options":

STANDALONE build (where value is cached, "the original code" : https://github.com/llvm/llvm-project/blob/main/openmp/CMakeLists.txt#L23)

not STANDALONE (where value is not cached: https://github.com/llvm/llvm-project/blob/main/openmp/CMakeLists.txt#L86):

We have not provided any standalone build options yet so I think current code is aligned with other RTs.

libsycl/CMakeLists.txt

libsycl/README.md

libsycl/include/sycl/detail/config.hpp

libsycl/CMakeLists.txt

libsycl/include/sycl/detail/config.hpp

philnik777 · 2025-06-16T16:16:59Z

libsycl/src/version.hpp.in

src seems like a weird place for a file that'll be part of the installed headers.

Well, I think that we use this approach to store file that is just an input for generator because of the way we copy headers. Probably a good chance to discuss this approach too.
I noticed that other projects do copy per file instead of doing copy of whole include directory.
I think libcxx uses file list for headers to use copy_if_different tool that copies files only if they were changed. This approach allows to store input files in the same folder with full header files.
copy_directory_if_different is added in later version, in 3.26.
We use copy_directory that doesn't detect changes but works faster. At early stages I believe there is no noticeable difference.
Do you know if there is any other reason for other projects to copy headers per file?

I don't think so, though I wasn't there for that specific part of libc++'s history. Maybe @ldionne knows more.

libsycl/src/platform.cpp

KseniyaTikhomirova

philnik777 thank you for the review, I really appreciate your comments and guidance.

I replied to all your comments and will provide code updates a bit later.

KseniyaTikhomirova · 2025-06-17T11:02:58Z

libsycl/CMakeLists.txt

+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD_REQUIRED YES)
+set(CMAKE_CXX_EXTENSIONS OFF)


you are right, I will remove cache entry here, thank you.

libsycl/.clang-format

libsycl/CMakeLists.txt

libsycl/README.md

libsycl/CMakeLists.txt

libsycl/include/sycl/detail/config.hpp

KseniyaTikhomirova · 2025-06-17T17:10:44Z

libsycl/src/version.hpp.in

Well, I think that we use this approach to store file that is just an input for generator because of the way we copy headers. Probably a good chance to discuss this approach too.
I noticed that other projects do copy per file instead of doing copy of whole include directory.
I think libcxx uses file list for headers to use copy_if_different tool that copies files only if they were changed. This approach allows to store input files in the same folder with full header files.
copy_directory_if_different is added in later version, in 3.26.
We use copy_directory that doesn't detect changes but works faster. At early stages I believe there is no noticeable difference.
Do you know if there is any other reason for other projects to copy headers per file?

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

libsycl/include/sycl/sycl.hpp

libsycl/docs/index.rst

asudarsa

Looks good to me. Couple of minor nits.

Thanks

philnik777 · 2025-06-21T17:53:35Z

Also, it would be great if we could get the precommit CI running ASAP. IIUC the basic configurations would be:

libstdc++/GCC(?)
MSVC STL/MSVC
libunwind/libc++abi/libc++/Clang
That should be pretty lean on the CI resources and could probably easily run in the GH-provided runners for now.

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

libsycl/docs/index.rst

KseniyaTikhomirova · 2025-06-23T15:15:59Z

Also, it would be great if we could get the precommit CI running ASAP. IIUC the basic configurations would be:

libstdc++/GCC(?)

MSVC STL/MSVC

libunwind/libc++abi/libc++/Clang
That should be pretty lean on the CI resources and could probably easily run in the GH-provided runners for now.

Could you please clarify - is it a blocker for the current PR? Can we add it in the following PRs?

philnik777

Also, it would be great if we could get the precommit CI running ASAP. IIUC the basic configurations would be:

libstdc++/GCC(?)

MSVC STL/MSVC

libunwind/libc++abi/libc++/Clang
That should be pretty lean on the CI resources and could probably easily run in the GH-provided runners for now.

Could you please clarify - is it a blocker for the current PR? Can we add it in the following PRs?

I don't think it should be a blocker. I'd like it soon though, since it significantly increases confidence that code changes are correct.

libsycl/include/sycl/__detail/config.hpp

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

KseniyaTikhomirova · 2025-06-23T16:13:24Z

Also, it would be great if we could get the precommit CI running ASAP. IIUC the basic configurations would be:

libstdc++/GCC(?)

MSVC STL/MSVC

libunwind/libc++abi/libc++/Clang
That should be pretty lean on the CI resources and could probably easily run in the GH-provided runners for now.

Could you please clarify - is it a blocker for the current PR? Can we add it in the following PRs?

I don't think it should be a blocker. I'd like it soon though, since it significantly increases confidence that code changes are correct.

I agree, I will add libsycl build workflow as separate PR.

tahonermann

Thanks @KseniyaTikhomirova! I unresolved one previous comment to follow up some more and added a few other minor comments.

tahonermann · 2025-06-23T21:44:43Z

libsycl/src/CMakeLists.txt

+  add_library(${LIB_NAME} SHARED
+              $<TARGET_OBJECTS:${LIB_OBJ_NAME}>)


Is this add_library call specifying that the shared library is to be created using the same object files that were linked into the static library? In general, objects suitable for bundling in a static library are not necessarily suitable for bundling in a shared library (e.g., if PIC isn't enabled). I think separate objects files should be built for each with, perhaps, an opt-in to use the same objects for each where that is known to work. I briefly looked for evidence of other LLVM projects doing this and didn't find any.

It's an object library above. I'm not sure why we need an object library and then a shared library, usually object libraries are used for some indirection before creating the final library but seems like there's just one step here.

This split is done for SYCL RT unittests (UT) target that is not included to this PR.

we do not provide SYCL RT static lib and don't plan to

fPIC option is set by default in LLVM (see LLVM_ENABLE_PIC)

the usage is that we have object library sycl_object. It is used for shared library "sycl" creation and for UT executable creation (not included to this PR).

tahonermann · 2025-06-23T21:47:10Z

libsycl/src/CMakeLists.txt

+    target_compile_options(${LIB_OBJ_NAME} PRIVATE ${ARG_COMPILE_OPTIONS})
+  endif()
+
+  cxx_add_warning_flags(${LIB_OBJ_NAME} ${LIBSYCL_ENABLE_WERROR} ${LIBSYCL_ENABLE_PEDANTIC})


I think the cxx_add_warning_flags call would be better colocated with the target_compile_definitions call above.

An additional call to cxx_add_warning_flags might be needed depending on how we resolve my other comment about sharing objects between the static and shared library construction (which assumes I know what I'm talking about which might not be the case).

moved above 83458d1

tahonermann · 2025-06-23T22:03:13Z

libsycl/src/CMakeLists.txt

+  add_library(sycl ALIAS ${LIB_NAME})
+endif()
+
+if (CMAKE_SYSTEM_NAME STREQUAL Windows)


Why is this checking for CMAKE_SYSTEM_NAME rather than just checking if WIN32 is true?

updated 83458d1

asudarsa · 2025-06-23T23:45:35Z

libsycl/.clang-format

@@ -1 +1,4 @@
 BasedOnStyle: LLVM
+


Thinking if libsycl directory should reside here
https://github.com/llvm/llvm-project/tree/main/offload

Thanks

jhuber6 · 2025-06-24T12:44:54Z

libsycl/src/CMakeLists.txt

+  add_library(${LIB_NAME} SHARED
+              $<TARGET_OBJECTS:${LIB_OBJ_NAME}>)


It's an object library above. I'm not sure why we need an object library and then a shared library, usually object libraries are used for some indirection before creating the final library but seems like there's just one step here.

jhuber6 · 2025-06-24T13:45:53Z

libsycl/CMakeLists.txt

+set(LIBSYCL_BUILD_INCLUDE_DIR ${LLVM_BINARY_DIR}/${LIBSYCL_INCLUDE_DIR})
+set(LIBSYCL_SOURCE_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/include)
+
+set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBSYCL_LIBRARY_DIR})


I prefer this as a property on the library, but it's not a bit deal.

I aligned this place with the libcxx approach. Since property is set based on CMAKE_ARCHIVE_OUTPUT_DIRECTORY and is implicitly applied to all corresponding targets (one for now) it should be ok.

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

[SYCL] Add libsycl, a SYCL RT library implementation project (#3)

5bbfeca

This patch introduces libsycl, a SYCL runtime library implementation, as a top-level LLVM runtime project. It contains the basic folder layout and CMake infrastructure to build a dummy SYCL library.

KseniyaTikhomirova requested a review from a team as a code owner June 16, 2025 15:38

llvmbot added the cmake Build system in general and CMake in particular label Jun 16, 2025

philnik777 reviewed Jun 16, 2025

View reviewed changes

KseniyaTikhomirova commented Jun 17, 2025

View reviewed changes

Fznamznon added the SYCL https://registry.khronos.org/SYCL label Jun 18, 2025

KseniyaTikhomirova added 2 commits June 18, 2025 05:59

address comments

fdf4ddb

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

rename detail folder

fb11e68

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

KseniyaTikhomirova requested review from AlexeySachkov and philnik777 June 18, 2025 14:23

asudarsa requested review from asudarsa and removed request for AlexeySachkov June 18, 2025 15:00

asudarsa reviewed Jun 18, 2025

View reviewed changes

libsycl/include/sycl/sycl.hpp Show resolved Hide resolved

asudarsa reviewed Jun 18, 2025

View reviewed changes

libsycl/docs/index.rst Show resolved Hide resolved

asudarsa approved these changes Jun 18, 2025

View reviewed changes

fix clang format, update docs

e901d30

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

asudarsa reviewed Jun 23, 2025

View reviewed changes

libsycl/docs/index.rst Outdated Show resolved Hide resolved

philnik777 reviewed Jun 23, 2025

View reviewed changes

libsycl/include/sycl/__detail/config.hpp Outdated Show resolved Hide resolved

bader self-requested a review June 23, 2025 15:40

fix more comments

2caf523

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

philnik777 approved these changes Jun 23, 2025

View reviewed changes

tahonermann reviewed Jun 23, 2025

View reviewed changes

asudarsa reviewed Jun 23, 2025

View reviewed changes

jhuber6 reviewed Jun 24, 2025

View reviewed changes

cmake files update

83458d1

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

KseniyaTikhomirova requested a review from tahonermann June 24, 2025 14:48

		add_library(${LIB_NAME} SHARED
		$<TARGET_OBJECTS:${LIB_OBJ_NAME}>)

[SYCL] Add libsycl, a SYCL RT library implementation project #144372

Are you sure you want to change the base?

[SYCL] Add libsycl, a SYCL RT library implementation project #144372

Uh oh!

Conversation

KseniyaTikhomirova commented Jun 16, 2025

Uh oh!

github-actions bot commented Jun 16, 2025

Uh oh!

KseniyaTikhomirova commented Jun 16, 2025

Uh oh!

philnik777 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

KseniyaTikhomirova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

asudarsa left a comment

Choose a reason for hiding this comment

Uh oh!

philnik777 commented Jun 21, 2025

Uh oh!

Uh oh!

KseniyaTikhomirova commented Jun 23, 2025

Uh oh!

philnik777 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

KseniyaTikhomirova commented Jun 23, 2025

Uh oh!

tahonermann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhuber6 Jun 24, 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!

jhuber6 Jun 24, 2025 •

edited

Loading

jhuber6 Jun 24, 2025 •

edited

Loading