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

philnik777 · 2025-06-16T15:58:01Z

libsycl/CMakeLists.txt

+
+# Enable all warnings by default
+if(MSVC)
+  append("/W4" CMAKE_CXX_FLAGS)
+else()
+  append("-Wall -Wextra" CMAKE_CXX_FLAGS)
+endif()
+
+if(LIBSYCL_ENABLE_WERROR)
+  if(MSVC)
+    append("/WX" CMAKE_CXX_FLAGS)
+  else()
+    append("-Werror" CMAKE_CXX_FLAGS)
+  endif()
+endif()
+


We have runimtes/Modules/WarningFlags.cmake. Maybe that should be used here?

thanks for highlighting this, I will update it

philnik777 · 2025-06-16T15:59:20Z

libsycl/CMakeLists.txt

+#===============================================================================
+
+# Enable all warnings by default
+if(MSVC)


Is the plan to support MSVC as a compiler? AFAIK no other runtime does that currently.

yes, we support MSVC.

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

libsycl/README.md

+
+#### How to build
+
+TBD


I guess this will be filled in future PRs? Maybe Make this a TODO instead? That's usually easier to grep for.

Yes, this is planned to be filled in future PRs, added some sections to show that the project will be properly documented.
I can replace "TBD" with "TODO", of course, although I don't see any difference in grepping by "TBD" or "TODO".
Please let me know your thoughts. Thanks.

philnik777 · 2025-06-16T16:00:46Z

libsycl/include/sycl/detail/config.hpp

+///
+//===----------------------------------------------------------------------===//
+
+#ifndef __LIBSYCL_DETAIL_CONFIG_HPP


Is there a reason you use double underscores everywhere? That style tends to be use by compiler-defined stuff, so we avoid it in libc++.

good catch, thanks. we have some macro like "__SYCL_DEVICE_ONLY" that have to have double underscore because it is defined by compiler. And it should be kept this way.
But the rest, the macro you are pointing to - don't need double underscores. I will update it, thank you!

philnik777 · 2025-06-16T16:05:14Z

libsycl/include/sycl/detail/config.hpp

+#define __LIBSYCL_BEGIN_UNVERSIONED_NAMESPACE namespace sycl {
+#define __LIBSYCL_END_UNVERSIONED_NAMESPACE }
+
+#define __LIBSYCL_BEGIN_VERSIONED_NAMESPACE                                    \


Maybe name this BEGIN_NAMESPACE_SYCL instead? Otherwise it's not really clear what namespace you're opening.

Makes sense, I will rename them to the following:
_LIBSYCL_BEGIN_UNVERSIONED_NAMESPACE_SYCL
_LIBSYCL_BEGIN_NAMESPACE_SYCL

Thank you.

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

libsycl/CMakeLists.txt

+set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBSYCL_LIBRARY_DIR})
+set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBSYCL_LIBRARY_DIR})
+
+set(LIBSYCL_MAJOR_VERSION 0)


I assume libsycl should only become ABI stable once the major version is bumped to 1?

Yes, there is a long way of upstreaming effort and major version will be updated with meaningful value later.

philnik777 · 2025-06-16T16:09:13Z

libsycl/include/sycl/detail/config.hpp

+
+#else // _WIN32
+
+#define __LIBSYCL_DLL_LOCAL __attribute__((visibility("hidden")))


Suggested change

#define __LIBSYCL_DLL_LOCAL __attribute__((visibility("hidden")))

#define __LIBSYCL_DLL_LOCAL __attribute__((__visibility__("hidden")))

Is there a reason to use __attribute__ instead of [[]]?

I don't think so. Both versions have gnu specific view (__attribute__((visibility("hidden"))) vs [[gnu::visibility("hidden")]]). It looks like both versions are used among llvm-project subprojects. By any chance do you know if there is any preference?

AFAIK visibility and __visibility__ are both valid.

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?

philnik777 · 2025-06-16T16:17:37Z

libsycl/src/platform.cpp

+
+#include <sycl/platform.hpp>
+
+#include <stdexcept>


Which standard libraries do you plan to support?

libstdc++, MSVC STL and we will have experimental support for building and linking sycl runtime with libc++ library instead of libstdc++.

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.

KseniyaTikhomirova · 2025-06-17T11:07:16Z

libsycl/.clang-format

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


I don't see it to be set in any other runtime (and any other project either). It doesn't seem right to differ in style from other runtimes.

KseniyaTikhomirova · 2025-06-17T11:11:17Z

libsycl/CMakeLists.txt

+set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBSYCL_LIBRARY_DIR})
+set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBSYCL_LIBRARY_DIR})
+
+set(LIBSYCL_MAJOR_VERSION 0)


Yes, there is a long way of upstreaming effort and major version will be updated with meaningful value later.

KseniyaTikhomirova · 2025-06-17T11:40:20Z

libsycl/CMakeLists.txt

+#===============================================================================
+
+# Enable all warnings by default
+if(MSVC)


yes, we support MSVC.

KseniyaTikhomirova · 2025-06-17T11:43:59Z

libsycl/README.md

+
+#### How to build
+
+TBD


Yes, this is planned to be filled in future PRs, added some sections to show that the project will be properly documented.
I can replace "TBD" with "TODO", of course, although I don't see any difference in grepping by "TBD" or "TODO".
Please let me know your thoughts. Thanks.

KseniyaTikhomirova · 2025-06-17T16:47:10Z

libsycl/CMakeLists.txt

+
+# Enable all warnings by default
+if(MSVC)
+  append("/W4" CMAKE_CXX_FLAGS)
+else()
+  append("-Wall -Wextra" CMAKE_CXX_FLAGS)
+endif()
+
+if(LIBSYCL_ENABLE_WERROR)
+  if(MSVC)
+    append("/WX" CMAKE_CXX_FLAGS)
+  else()
+    append("-Werror" CMAKE_CXX_FLAGS)
+  endif()
+endif()
+


thanks for highlighting this, I will update it

KseniyaTikhomirova · 2025-06-17T16:51:00Z

libsycl/include/sycl/detail/config.hpp

+///
+//===----------------------------------------------------------------------===//
+
+#ifndef __LIBSYCL_DETAIL_CONFIG_HPP


good catch, thanks. we have some macro like "__SYCL_DEVICE_ONLY" that have to have double underscore because it is defined by compiler. And it should be kept this way.
But the rest, the macro you are pointing to - don't need double underscores. I will update it, thank you!

KseniyaTikhomirova · 2025-06-17T16:56:02Z

libsycl/include/sycl/detail/config.hpp

+#define __LIBSYCL_BEGIN_UNVERSIONED_NAMESPACE namespace sycl {
+#define __LIBSYCL_END_UNVERSIONED_NAMESPACE }
+
+#define __LIBSYCL_BEGIN_VERSIONED_NAMESPACE                                    \


Makes sense, I will rename them to the following:
_LIBSYCL_BEGIN_UNVERSIONED_NAMESPACE_SYCL
_LIBSYCL_BEGIN_NAMESPACE_SYCL

Thank you.

KseniyaTikhomirova · 2025-06-17T16:59:12Z

libsycl/include/sycl/detail/config.hpp

+
+#if __LIBSYCL_BUILD_SYCL_DLL
+#define __LIBSYCL_EXPORT __declspec(dllexport)
+#define __LIBSYCL_EXPORT_DEPRECATED(x) __declspec(dllexport, deprecated(x))


in our codebase we have 1 exported deprecated function but I will try to get rid of it (will avoid exporting it) since ABI is not fixed yet in upstreaming version of SYCL.

I will remove this macro completely for now. Thanks.

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 <kseniya.tikhomirova@intel.com>

asudarsa · 2025-06-18T15:07:52Z

libsycl/include/sycl/sycl.hpp

+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+


Minor nit. May be a small file description:
//===----------------------------------------------------------------------===//
///
/// \file
/// This file is a SYCL2020 standard header file.
///
//===----------------------------------------------------------------------===//

asudarsa · 2025-06-18T15:12:09Z

libsycl/docs/index.rst

@@ -0,0 +1,69 @@
+.. _index:


This doc can be reformatted as follows:

Main Heading: SYCL runtime implementation
Sub-topics:
1 .Current Status
2. Overview
3. Build steps
Thanks

asudarsa

Looks good to me. Couple of minor nits.

Thanks

[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 <kseniya.tikhomirova@intel.com>

rename detail folder

fb11e68

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

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

asudarsa approved these changes Jun 18, 2025

View reviewed changes


		#else // _WIN32

		#define __LIBSYCL_DLL_LOCAL __attribute__((visibility("hidden")))


		#### How to build

		TBD


		#include <sycl/platform.hpp>

		#include <stdexcept>


		#### How to build

		TBD

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

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 Jun 17, 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KseniyaTikhomirova 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

Uh oh!

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

KseniyaTikhomirova Jun 17, 2025 •

edited

Loading

KseniyaTikhomirova Jun 17, 2025 •

edited

Loading