[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.

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.

libsycl/CMakeLists.txt

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.

libsycl/src/CMakeLists.txt

libsycl/.clang-format

libsycl/src/CMakeLists.txt

libsycl/CMakeLists.txt

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

asudarsa · 2025-06-27T17:40:29Z

Hi @KseniyaTikhomirova
Can you please take a look at the merge conflict reported here?
Thanks

jhuber6

Should be a fine starting point for now, we can fix things later.

bader · 2025-06-27T17:59:12Z

Hi @KseniyaTikhomirova Can you please take a look at the merge conflict reported here? Thanks

@asudarsa, resolved.

tahonermann

Thank you for responding to my prior comments. I added a number of new comments, many of which are minor. I would really like to see some of the differences between windows and Linux/UNIX build differences eliminated; particularly the library naming differences and the "d" debug postfix naming. The default build should produce binaries named appropriately for a LLVM/Clang monorepo build and distribution (which shouldn't differ much between Windows and Linux/UNIX).

libsycl/.clang-format

tahonermann · 2025-06-30T02:37:39Z

libsycl/include/sycl/__detail/config.hpp

+
+#      define _LIBSYCL_DLL_LOCAL
+
+#      if _LIBSYCL_BUILD_SYCL_DLL


I think this should be #ifdef. libc++ uses _LIBCPP_BUILDING_LIBRARY as its name for this macro; I suggest following suit.

Suggested change

# if _LIBSYCL_BUILD_SYCL_DLL

# ifdef _LIBSYCL_BUILDING_LIBRARY

will update

tahonermann · 2025-06-30T02:55:27Z

libsycl/CMakeLists.txt

+  if (MSVC)
+    list(APPEND LIBSYCL_RT_LIBS ${LIBSYCL_SHARED_OUTPUT_NAME}${LIBSYCL_MAJOR_VERSION}d)
+  endif()


Is MSVC being used as a proxy for detection of a multiconfiguration build so that both release and debug libraries are built? If so, that doesn't seem right. Note that the corresponding set_target_properties command in libsycl/src/CMakeLists.txt to set DEBUG_POSTFIX is not contingent on MSVC.

MSVC provides two incompatible build variants for its CRT: release and debug.
To avoid potential issues in user code we also need to provide two kinds of SYCL Runtime Library for release and debug configurations. This part was removed from this CMakeLists.txt for initial stages. I will remove these lines also. It is redundant now.

extra lib/target generation is removed 6e47300

tahonermann · 2025-06-30T02:59:26Z

libsycl/src/CMakeLists.txt

+if (WIN32)
+  add_library(sycl ALIAS ${LIB_NAME})
+
+  set_target_properties(${LIB_NAME} PROPERTIES DEBUG_POSTFIX d)


None of the other LLVM projects appear to set DEBUG_POSTFIX. Is it really desirable for libsycl? Why only for Windows?

this part is related to the same problem/feature with CRT & release vs debug build on Windows. Will remove for now.

Although we still need to add "d" postfix, I will align impl with libcxx https://github.com/llvm/llvm-project/blob/main/libcxx/CMakeLists.txt#L685

That sounds good. The DEBUG_POSTFIX property approach might be a better way to handle the actual naming; it just needs to be done with the right set of conditional checks (e.g., CMAKE_MSVC_RUNTIME_LIBRARY indicating the debug implementation). Doing what libc++ does seems reasonable to me though.

updated conditions to set "d" suffix properly. As reference I used https://github.com/intel/llvm/pull/18200/files#diff-8b7c97dbec3e02fda06bd9374dde5ff0f454f6d76b6a620236a9e84de71f08ea that is on review now.

done in 6e47300

tahonermann · 2025-06-30T03:01:48Z

libsycl/src/CMakeLists.txt

+endif()
+
+# Version-agnostic name of the import library, has effect on Windows only.
+set(IMPLIB_NAME "sycl")


I know it is customary for import libraries to drop a leading "lib" prefix when the corresponding library is named, e.g., "libfoo". In this case, the name of the project is "libsycl", so perhaps it should be preserved here?

Suggested change

set(IMPLIB_NAME "sycl")

set(IMPLIB_NAME "libsycl")

To be honest I don't see any benefits of setting name with "lib" prefix explicitly. What are they?

Use of a different name could help to avoid an unintended use of the DPC++ import library that has the same name.

updated 6e47300

@tahonermann want to reiterate this discussion. I kept sycl.lib name.
AFAIU libc++ uses c++.dll + c++.lib for dynamic & import libraries and libc++.lib for static library. That means that if we want to align with them - we don't need any extra "lib" prefix for SYCL RT. we don't have static version of library and don't plan to.

installed files look like:
Linux:

-- Up-to-date: <local dir>/install/release/include/sycl/version.hpp -- Up-to-date: <local dir>/install/release/include/sycl -- Up-to-date: <local dir>/install/release/include/sycl/sycl.hpp -- Up-to-date: <local dir>/install/release/include/sycl/__detail -- Up-to-date: <local dir>/install/release/include/sycl/__detail/config.hpp -- Up-to-date: <local dir>/install/release/include/sycl/platform.hpp -- Up-to-date: <local dir>/install/release/lib/libsycl.so.0.1.0 -- Up-to-date: <local dir>/install/release/lib/libsycl.so

Windows:

-- Installing: <local dir>/install/release/include/sycl/version.hpp -- Up-to-date: <local dir>/install/release/include/sycl -- Installing: <local dir>/install/release/include/sycl/platform.hpp -- Installing: <local dir>/install/release/include/sycl/sycl.hpp -- Installing: <local dir>/install/release/include/sycl/__detail -- Installing: <local dir>/install/release/include/sycl/__detail/config.hpp -- Installing: <local dir>/install/release/lib/sycl.lib -- Installing: <local dir>/install/release/bin/sycl.dll

tahonermann · 2025-06-30T03:12:20Z

libsycl/src/CMakeLists.txt

+if (WIN32)
+set(LIB_NAME "sycl${LIBSYCL_MAJOR_VERSION}")
+else()
+set(LIB_NAME "sycl")
+endif()


I don't think it makes sense for the DLL name to include version information by default. I don't think other LLVM projects embed version information in their respective DLL names for windows/MSVC builds. I can appreciate that a versioned name might be desired as part of an installable run-time component, but here we're just building as part of the monorepo, so the version information isn't helpful and the inconsistent naming doesn't seem desirable.

I think it should be possible for someone building LLVM to override the default library name though. Perhaps this should be a cached setting.

removed in b824da0

tahonermann · 2025-06-30T03:17:02Z

libsycl/src/CMakeLists.txt

+    endif()
+  endif()
+
+  if (MSVC)


The else block suggests this should be checking WIN32, not MSVC specifically.

Suggested change

if (MSVC)

if (WIN32)

I will update all similar places. These difference in intel/llvm came from intel/llvm#7859, a specific build environment. I do agree that these details may be not really needed at this point.

updated in 6e47300

Basically MSVC & WIN32 are not completely equal. MSVC & WIN32 provide same values for LLVM/Clang &
Microsoft Visual C/C++ Compiler. But MSVC is 0 for GCC (available as MinGW).
In intel/llvm there was PR that introduces the difference in usage of WIn32 and MSVC (mentioned in the previous reply). Although we don't test and don't guarantee it to work.
So I eliminated that difference in this PR and added protection against untested and unsupported usage. This decision can be revisited later.
One more note, I replaced WIN32 check with "CMAKE_SYSTEM_NAME STREQUAL Windows" check due to "soft" deprecation of WIN32: https://discourse.cmake.org/t/platform-id-vs-win32-vs-cmake-system-name/1226

tahonermann · 2025-06-30T03:21:03Z

libsycl/src/CMakeLists.txt

+  set_target_properties(${LIB_NAME} PROPERTIES LINKER_LANGUAGE CXX)
+
+  if (WIN32)
+    target_link_libraries(${LIB_NAME} PRIVATE shlwapi)


The dependency on shlwapi probably isn't needed at this time.

Suggested change

target_link_libraries(${LIB_NAME} PRIVATE shlwapi)

It is not needed in this PR since we have no code. But it will be used at pretty early stage. Why can't we declare now that it will be a dependency?

There is a chance that the dependency won't be needed once the code that currently requires it is put up for review. LLVM already provides abstractions for many of the features provided by shlwapi. Depending on what functionality is needed, it might make more sense to add an abstraction for it in one of the low-level LLVM libraries like llvm/Support.

removed in 6e47300#diff-012c1c75642ff105a8083560c92d539c11a7d62cbe4946820528c77ba8875270

tahonermann · 2025-06-30T03:38:34Z

libsycl/src/ld-version-script.txt

+  /* Do not use extern "C++" matcher for C++ functions,                     */
+  /* because vtable and typeinfo symbols make extern "C++" patterns more    */
+  /* complicated than patterns against mangled names.                       */
+  /*                                                                        */
+  /* With extern "C++" we have to match for "vtable for sycl::foo", but */
+  /* not match for "vtable for std::__internal<sycl::foo>".             */


Suggested change

/* Do not use extern "C++" matcher for C++ functions, */

/* because vtable and typeinfo symbols make extern "C++" patterns more */

/* complicated than patterns against mangled names. */

/* */

/* With extern "C++" we have to match for "vtable for sycl::foo", but */

/* not match for "vtable for std::__internal<sycl::foo>". */

/* Symbols to be exported are selected based on mangled names rather than */

/* the demangled names provided by the `extern "C++"` matcher because it is */

/* easy to express "export everything defined in the sycl namespace" using */

/* the former. Matching demangled names is more complicated in the presence */

/* of examples like: */

/* "vtable for sycl::foo" (should be exported) */

/* "vtable for std::__internal<sycl::foo>" (should not be exported) */

will update, thanks

updated in 6e47300#diff-012c1c75642ff105a8083560c92d539c11a7d62cbe4946820528c77ba8875270

tahonermann · 2025-06-30T03:43:11Z

libsycl/src/CMakeLists.txt

+  add_library(${LIB_OBJ_NAME} OBJECT ${ARG_SOURCES})
+
+  # Common compilation step setup
+  target_compile_definitions(${LIB_OBJ_NAME} PRIVATE $<$<BOOL:${MSVC}>:_LIBSYCL_BUILD_SYCL_DLL>)


The dependency on MSVC doesn't seem right here. I think _LIBSYCL_BUILD_SYCL_DLL should be defined for any compiler targeting Windows given that the check for this macro in libsycl/include/sycl/__detail/config.hpp is target (not compiler) specific.

will update

updated in 6e47300#diff-012c1c75642ff105a8083560c92d539c11a7d62cbe4946820528c77ba8875270

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

libsycl/src/CMakeLists.txt Outdated Show resolved Hide resolved

libsycl/src/CMakeLists.txt Outdated Show resolved Hide resolved

libsycl/src/CMakeLists.txt Show resolved Hide resolved

asudarsa reviewed Jun 23, 2025

View reviewed changes

libsycl/.clang-format Show resolved Hide resolved

jhuber6 reviewed Jun 24, 2025

View reviewed changes

libsycl/src/CMakeLists.txt Outdated Show resolved Hide resolved

libsycl/CMakeLists.txt Show resolved Hide resolved

cmake files update

83458d1

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

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

asudarsa requested review from jhuber6 and AlexeySachkov June 27, 2025 17:40

jhuber6 approved these changes Jun 27, 2025

View reviewed changes

Merge branch 'main' into reviewed_addlibsycl

97752d6

tahonermann requested changes Jun 30, 2025

View reviewed changes

KseniyaTikhomirova added 5 commits July 1, 2025 12:42

Merge branch 'llvm:main' into reviewed_addlibsycl

8811e0d

fix cmake related comments

6e47300

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

fix win build

2573733

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

remove version from win library name

554decd

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

Merge branch 'main' into reviewed_addlibsycl

e8e2ca9

KseniyaTikhomirova marked this pull request as draft July 4, 2025 12:33

rest of the changes for version removal

b824da0

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

KseniyaTikhomirova marked this pull request as ready for review July 4, 2025 15:49

KseniyaTikhomirova requested a review from tahonermann July 4, 2025 15:51

remove extra code for implib name

81c9efd

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

	# if _LIBSYCL_BUILD_SYCL_DLL
	# ifdef _LIBSYCL_BUILDING_LIBRARY

-  /* Do not use extern "C++" matcher for C++ functions,                     */
-  /* because vtable and typeinfo symbols make extern "C++" patterns more    */
-  /* complicated than patterns against mangled names.                       */
-  /*                                                                        */
-  /* With extern "C++" we have to match for "vtable for sycl::foo", but */
-  /* not match for "vtable for std::__internal<sycl::foo>".             */
+  /* Symbols to be exported are selected based on mangled names rather than   */
+  /* the demangled names provided by the `extern "C++"` matcher because it is */
+  /* easy to express "export everything defined in the sycl namespace" using  */
+  /* the former. Matching demangled names is more complicated in the presence */
+  /* of examples like:                                                        */
+  /*   "vtable for sycl::foo"                  (should be exported)           */
+  /*   "vtable for std::__internal<sycl::foo>" (should not be exported)       */

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

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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asudarsa commented Jun 27, 2025

Uh oh!

jhuber6 left a comment

Choose a reason for hiding this comment

Uh oh!

bader commented Jun 27, 2025

Uh oh!

tahonermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

KseniyaTikhomirova Jul 2, 2025 •

edited

Loading

KseniyaTikhomirova Jul 2, 2025 •

edited

Loading