Skip to content

[SYCL][NewOffloadModel] Update SYCL device library generation for new offloading model #20512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
+99 −61
Closed

[SYCL][NewOffloadModel] Update SYCL device library generation for new offloading model #20512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7d6b80f
Add nick's changes=
YixingZhang007 Oct 29, 2025
67f0939
modify the commands in kernal_from_file.cpp and this test is now passing
YixingZhang007 Oct 30, 2025
dca2051
clean up the test command in kernel_from_file.cpp
YixingZhang007 Oct 30, 2025
94d03ca
change the test command for SeperateCompiler/test.cpp, now this test …
YixingZhang007 Oct 30, 2025
324d02a
add support for backward compatibility
YixingZhang007 Nov 2, 2025
b1a8f8d
update the test to test backward compatibility
YixingZhang007 Nov 2, 2025
9dfd1af
code clean up and resolve CI failure
YixingZhang007 Nov 2, 2025
89b35d5
revert the changes for enabling new offloading model by default
YixingZhang007 Nov 5, 2025
ff7ea74
resolve the CI failure by updating the .o to be .old.o in sycl_noinli…
YixingZhang007 Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions clang/lib/Driver/ToolChains/SYCL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ getDeviceLibrariesLegacy(const Compilation &C, const llvm::Triple &TargetTriple,
StringRef LibSuffix = ".bc";
if (IsNewOffload)
// For new offload model, we use packaged .bc files.
Copy link
Contributor

@srividya-sundaram srividya-sundaram Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the comment since we now use object files and not bitcode files.

LibSuffix = IsWindowsMSVCEnv ? ".new.obj" : ".new.o";
LibSuffix = IsWindowsMSVCEnv ? ".obj" : ".o";
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, before this PR, old offloading model used name.ext library name pattern and new offloading model used name.new.ext library name pattern.
After the change as far as I can see both old and new offloading model use name.ext library.
Or what am I missing?

Copy link
Contributor Author

@YixingZhang007 YixingZhang007 Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing this out! Yes, you are right that before this PR, the old offloading model used name.ext and the new offloading model used name.new.ext. Currently, name.ext is built with the old offloading model and name.new.ext is built with the new offloading model. However, if we enable the new offloading model by default in the future, both name.ext and name.new.ext will be built with the new offloading model.

In this PR, we update name.ext to be built with the new offloading model, and we introduce name.old.ext which will be built with the old offloading model. As we can see from the testing so far, when we are compiling tests with the old offloading model and linking the name.ext (which is built with the new offloading model), the tests are working fine. However, we still want to keep name.old.ext in case users still want to interact with the old offloading model, such as in some of the tests where they want to extract the device code from the fat object file, which is only supported for the object file built with the old offloading model.

Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, why not keep the names as is, and when we enable new offloading model by default, just stop using name.new.ext?

auto addLibraries = [&](const SYCLDeviceLibsList &LibsList) {
for (const DeviceLibOptInfo &Lib : LibsList) {
if (!DeviceLibLinkInfo[Lib.DeviceLibOption])
Expand Down Expand Up @@ -807,7 +807,7 @@ SYCL::getDeviceLibraries(const Compilation &C, const llvm::Triple &TargetTriple,
StringRef LibSuffix = ".bc";
if (IsNewOffload)
// For new offload model, we use packaged .bc files.
Copy link
Contributor

@srividya-sundaram srividya-sundaram Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above.

LibSuffix = IsWindowsMSVCEnv ? ".new.obj" : ".new.o";
LibSuffix = IsWindowsMSVCEnv ? ".obj" : ".o";
auto addLibraries = [&](const SYCLDeviceLibsList &LibsList) {
for (const StringRef &Lib : LibsList) {
LibraryList.push_back(Args.MakeArgString(Twine(Lib) + LibSuffix));
Expand Down Expand Up @@ -1024,10 +1024,10 @@ const char *SYCL::Linker::constructLLVMLinkCommand(
const bool IsSYCLNativeCPU =
this->getToolChain().getTriple().isNativeCPU();
StringRef LibPostfix = ".bc";
StringRef NewLibPostfix = ".new.o";
StringRef NewLibPostfix = ".o";
if (HostTC->getTriple().isWindowsMSVCEnvironment() &&
C.getDriver().IsCLMode())
NewLibPostfix = ".new.obj";
NewLibPostfix = ".obj";
std::string FileName = this->getToolChain().getInputFilename(II);
StringRef InputFilename = llvm::sys::path::filename(FileName);
// NativeCPU links against libclc (libspirv)
Expand Down
56 changes: 29 additions & 27 deletions libdevice/cmake/modules/SYCLLibdevice.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
include(CheckCXXCompilerFlag)
set(obj_binary_dir "${CMAKE_LIBRARY_OUTPUT_DIRECTORY}")
set(obj-new-offload_binary_dir "${CMAKE_LIBRARY_OUTPUT_DIRECTORY}")
set(obj-old-offload_binary_dir "${CMAKE_LIBRARY_OUTPUT_DIRECTORY}")
if (MSVC)
set(obj-suffix obj)
set(obj-new-offload-suffix new.obj)
set(obj-old-offload-suffix old.obj)
set(spv_binary_dir "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}")
set(install_dest_spv bin)
set(devicelib_host_static_obj sycl-devicelib-host.lib)
set(devicelib_host_static_obj-new-offload sycl-devicelib-host.new.lib)
set(devicelib_host_static_obj-old-offload sycl-devicelib-host.old.lib)
else()
set(obj-suffix o)
set(obj-new-offload-suffix new.o)
set(obj-old-offload-suffix old.o)
set(spv_binary_dir "${CMAKE_LIBRARY_OUTPUT_DIRECTORY}")
set(install_dest_spv lib${LLVM_LIBDIR_SUFFIX})
set(devicelib_host_static_obj libsycl-devicelib-host.a)
set(devicelib_host_static_obj-new-offload libsycl-devicelib-host.new.a)
set(devicelib_host_static_obj-old-offload libsycl-devicelib-host.old.a)
endif()
set(spv-suffix spv)
set(bc-suffix bc)
set(bc_binary_dir "${CMAKE_LIBRARY_OUTPUT_DIRECTORY}")
set(install_dest_obj lib${LLVM_LIBDIR_SUFFIX})
set(install_dest_obj-new-offload lib${LLVM_LIBDIR_SUFFIX})
set(install_dest_obj-old-offload lib${LLVM_LIBDIR_SUFFIX})
set(install_dest_bc lib${LLVM_LIBDIR_SUFFIX})

string(CONCAT sycl_targets_opt
Expand Down Expand Up @@ -80,8 +80,8 @@ endif()

add_custom_target(libsycldevice)

set(filetypes obj obj-new-offload spv bc)
set(filetypes_no_spv obj obj-new-offload bc)
set(filetypes obj obj-old-offload spv bc)
set(filetypes_no_spv obj obj-old-offload bc)

foreach(filetype IN LISTS filetypes)
add_custom_target(libsycldevice-${filetype})
Expand Down Expand Up @@ -109,9 +109,9 @@ endif()

set(spv_device_compile_opts -fsycl-device-only -fsycl-device-obj=spirv)
set(bc_device_compile_opts -fsycl-device-only -fsycl-device-obj=llvmir)
set(obj-new-offload_device_compile_opts -fsycl -c --offload-new-driver
set(obj-old-offload_device_compile_opts -fsycl -c ${sycl_targets_opt} --no-offload-new-driver)
set(obj_device_compile_opts -fsycl -c --offload-new-driver
Copy link
Contributor

@sarnex sarnex Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my worry here is that it seems this PR enables the new offload model by default for some but not all compilation modes, and since we are setting the default device library files to be the new offloading model (since we pass --offload-new-driver), i am not sure of those compilation modes using the old offload model will be able to use libdevice. the new offload model supports using old libdevice files, but i had to explicitly implement that to get it working and i don't think anyone did the opposite case since it wasn't expected to be be needed, but it's possible it works automagically. can you make sure the cases using the old offload model still work?

Copy link
Contributor Author

@YixingZhang007 YixingZhang007 Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for raising this concern! I tried compiling a few SYCL E2E tests using the old offload model (by passing --no-offload-new-driver to clang++). The compilation appears to work correctly. For the old offload model, I found that llvm-link is called on libsycl-xxx.bc, whereas in the new offload model, libsycl-xxx.o is linked via the clang-linker-wrapper.
To further verify, I will remove the changes in this PR that enable the new offloading model by default and rerun the tests to see if any failures will occur in CI.

Copy link
Contributor

@sarnex sarnex Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, thanks!

Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to enable new offload model by default for some compilation modes to resolve the original problem that PR is trying to solve?

-foffload-lto=thin ${sycl_targets_opt})
set(obj_device_compile_opts -fsycl -c ${sycl_targets_opt})

# Compiles and installs a single device library.
#
Expand Down Expand Up @@ -364,47 +364,50 @@ if (NOT MSVC AND UR_SANITIZER_INCLUDE_DIR)
-I${UR_SANITIZER_INCLUDE_DIR}
-I${CMAKE_CURRENT_SOURCE_DIR})

set(sanitizer_pvc_compile_opts_obj -fsycl -c
set(sanitizer_pvc_compile_opts_obj -fsycl -c --offload-new-driver
-foffload-lto=thin
${sanitizer_generic_compile_opts}
${sycl_pvc_target_opt}
-D__LIBDEVICE_PVC__)

set(sanitizer_cpu_compile_opts_obj -fsycl -c
set(sanitizer_cpu_compile_opts_obj -fsycl -c --offload-new-driver
-foffload-lto=thin
${sanitizer_generic_compile_opts}
${sycl_cpu_target_opt}
-D__LIBDEVICE_CPU__)

set(sanitizer_dg2_compile_opts_obj -fsycl -c
set(sanitizer_dg2_compile_opts_obj -fsycl -c --offload-new-driver
-foffload-lto=thin
${sanitizer_generic_compile_opts}
${sycl_dg2_target_opt}
-D__LIBDEVICE_DG2__)

set(sanitizer_pvc_compile_opts_bc ${bc_device_compile_opts}
set(sanitizer_pvc_compile_opts_bc ${bc_device_compile_opts} --offload-new-driver
-foffload-lto=thin
${sanitizer_generic_compile_opts}
-D__LIBDEVICE_PVC__)

set(sanitizer_cpu_compile_opts_bc ${bc_device_compile_opts}
set(sanitizer_cpu_compile_opts_bc ${bc_device_compile_opts} --offload-new-driver
-foffload-lto=thin
${sanitizer_generic_compile_opts}
-D__LIBDEVICE_CPU__)

set(sanitizer_dg2_compile_opts_bc ${bc_device_compile_opts}
set(sanitizer_dg2_compile_opts_bc ${bc_device_compile_opts} --offload-new-driver
-foffload-lto=thin
${sanitizer_generic_compile_opts}
-D__LIBDEVICE_DG2__)

set(sanitizer_pvc_compile_opts_obj-new-offload -fsycl -c --offload-new-driver
-foffload-lto=thin
set(sanitizer_pvc_compile_opts_obj-old-offload -fsycl -c --no-offload-new-driver
${sanitizer_generic_compile_opts}
${sycl_pvc_target_opt}
-D__LIBDEVICE_PVC__)

set(sanitizer_cpu_compile_opts_obj-new-offload -fsycl -c --offload-new-driver
-foffload-lto=thin
set(sanitizer_cpu_compile_opts_obj-old-offload -fsycl -c --no-offload-new-driver
${sanitizer_generic_compile_opts}
${sycl_cpu_target_opt}
-D__LIBDEVICE_CPU__)

set(sanitizer_dg2_compile_opts_obj-new-offload -fsycl -c --offload-new-driver
-foffload-lto=thin
set(sanitizer_dg2_compile_opts_obj-old-offload -fsycl -c --no-offload-new-driver
${sanitizer_generic_compile_opts}
${sycl_dg2_target_opt}
-D__LIBDEVICE_DG2__)
Expand Down Expand Up @@ -704,8 +707,7 @@ if (NOT WIN32)
add_imf_host_cxx_flags_compile_flags_if_supported("-fcf-protection=full")
endif()

set(obj-new-offload_host_compile_opts ${imf_host_cxx_flags} --offload-new-driver
-foffload-lto=thin)
set(obj-old-offload_host_compile_opts ${imf_host_cxx_flags} --no-offload-new-driver)
set(obj_host_compile_opts ${imf_host_cxx_flags})

foreach(datatype IN ITEMS fp32 fp64 bf16)
Expand Down Expand Up @@ -834,9 +836,9 @@ foreach(arch IN LISTS full_build_archs)
COMPONENT libsycldevice)
endforeach()

# Add host device imf libraries for obj and new offload objects.
# Add host device imf libraries for obj and old offload objects.
foreach(dtype IN ITEMS bf16 fp32 fp64)
foreach(ftype IN ITEMS obj obj-new-offload)
foreach(ftype IN ITEMS obj obj-old-offload)
set(tgt_name imf_fallback_${dtype}_host_${ftype})

add_lib_imf(fallback-imf-${dtype}-host
Expand Down Expand Up @@ -864,7 +866,7 @@ foreach(dtype IN ITEMS bf16 fp32 fp64)
endforeach()
endforeach()

foreach(ftype IN ITEMS obj obj-new-offload)
foreach(ftype IN ITEMS obj obj-old-offload)
add_custom_target(imf_host_${ftype}
DEPENDS ${${ftype}_binary_dir}/${devicelib_host_static_${ftype}})
add_custom_command(
Expand Down
20 changes: 10 additions & 10 deletions libdevice/test/sycl_noinline.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,52 +21,52 @@ RUN: llvm-dis %t.bc -o - | FileCheck %s

RUN: clang-offload-bundler -type=o -unbundle \
RUN: -targets=sycl-spir64-unknown-unknown \
RUN: -input=%libsycldevice_obj_dir/libsycl-fallback-cassert.o -output=%t.bc
RUN: -input=%libsycldevice_obj_dir/libsycl-fallback-cassert.old.o -output=%t.bc
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to do this renaming now? why not keep default offloading model used now without additional suffixes and new offloading model to keep using new suffixes?

RUN: llvm-dis %t.bc -o - | FileCheck %s

RUN: clang-offload-bundler -type=o -unbundle \
RUN: -targets=sycl-spir64-unknown-unknown \
RUN: -input=%libsycldevice_obj_dir/libsycl-fallback-cmath.o -output=%t.bc
RUN: -input=%libsycldevice_obj_dir/libsycl-fallback-cmath.old.o -output=%t.bc
RUN: llvm-dis %t.bc -o - | FileCheck %s

RUN: clang-offload-bundler -type=o -unbundle \
RUN: -targets=sycl-spir64-unknown-unknown \
RUN: -input=%libsycldevice_obj_dir/libsycl-fallback-cmath-fp64.o -output=%t.bc
RUN: -input=%libsycldevice_obj_dir/libsycl-fallback-cmath-fp64.old.o -output=%t.bc
RUN: llvm-dis %t.bc -o - | FileCheck %s

RUN: clang-offload-bundler -type=o -unbundle \
RUN: -targets=sycl-spir64-unknown-unknown \
RUN: -input=%libsycldevice_obj_dir/libsycl-fallback-complex.o -output=%t.bc
RUN: -input=%libsycldevice_obj_dir/libsycl-fallback-complex.old.o -output=%t.bc
RUN: llvm-dis %t.bc -o - | FileCheck %s

RUN: clang-offload-bundler -type=o -unbundle \
RUN: -targets=sycl-spir64-unknown-unknown \
RUN: -input=%libsycldevice_obj_dir/libsycl-fallback-complex-fp64.o -output=%t.bc
RUN: -input=%libsycldevice_obj_dir/libsycl-fallback-complex-fp64.old.o -output=%t.bc
RUN: llvm-dis %t.bc -o - | FileCheck %s

RUN: clang-offload-bundler -type=o -unbundle \
RUN: -targets=sycl-spir64-unknown-unknown \
RUN: -input=%libsycldevice_obj_dir/libsycl-crt.o -output=%t.bc
RUN: -input=%libsycldevice_obj_dir/libsycl-crt.old.o -output=%t.bc
RUN: llvm-dis %t.bc -o - | FileCheck %s

RUN: clang-offload-bundler -type=o -unbundle \
RUN: -targets=sycl-spir64-unknown-unknown \
RUN: -input=%libsycldevice_obj_dir/libsycl-cmath.o -output=%t.bc
RUN: -input=%libsycldevice_obj_dir/libsycl-cmath.old.o -output=%t.bc
RUN: llvm-dis %t.bc -o - | FileCheck %s

RUN: clang-offload-bundler -type=o -unbundle \
RUN: -targets=sycl-spir64-unknown-unknown \
RUN: -input=%libsycldevice_obj_dir/libsycl-cmath-fp64.o -output=%t.bc
RUN: -input=%libsycldevice_obj_dir/libsycl-cmath-fp64.old.o -output=%t.bc
RUN: llvm-dis %t.bc -o - | FileCheck %s

RUN: clang-offload-bundler -type=o -unbundle \
RUN: -targets=sycl-spir64-unknown-unknown \
RUN: -input=%libsycldevice_obj_dir/libsycl-complex.o -output=%t.bc
RUN: -input=%libsycldevice_obj_dir/libsycl-complex.old.o -output=%t.bc
RUN: llvm-dis %t.bc -o - | FileCheck %s

RUN: clang-offload-bundler -type=o -unbundle \
RUN: -targets=sycl-spir64-unknown-unknown \
RUN: -input=%libsycldevice_obj_dir/libsycl-complex-fp64.o -output=%t.bc
RUN: -input=%libsycldevice_obj_dir/libsycl-complex-fp64.old.o -output=%t.bc
RUN: llvm-dis %t.bc -o - | FileCheck %s

CHECK: target triple ={{.*}}spir64
Expand Down
20 changes: 16 additions & 4 deletions sycl/test-e2e/Config/kernel_from_file.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,28 @@
// As we are doing a separate device compilation here, we need to explicitly
// add the device lib instrumentation (itt_compiler_wrapper)
// RUN: %clangxx -Wno-error=ignored-attributes -DSYCL_DISABLE_FALLBACK_ASSERT %cxx_std_optionc++17 -fsycl-device-only -fno-sycl-dead-args-optimization -Xclang -fsycl-int-header=%t.h %s -o %t.bc -Xclang -verify-ignore-unexpected=note,warning -Wno-sycl-strict
// >> ---- unbundle compiler wrapper and asan device objects
// RUN: clang-offload-bundler -type=o -targets=sycl-spir64-unknown-unknown -input=%sycl_static_libs_dir/libsycl-itt-compiler-wrappers%obj_ext -output=%t_compiler_wrappers.bc -unbundle
// RUN: %if linux %{ clang-offload-bundler -type=o -targets=sycl-spir64-unknown-unknown -input=%sycl_static_libs_dir/libsycl-asan%obj_ext -output=%t_asan.bc -unbundle %}

Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to keep existing E2E tests as is, without changes to make sure backward compatibility.
And if you want to add new tests for new offloading model, please add them to https://github.com/intel/llvm/tree/sycl/sycl/test-e2e/NewOffloadDriver

// >> ---- link device code
// RUN: %if linux %{ llvm-link -o=%t_app.bc %t.bc %t_compiler_wrappers.bc %t_asan.bc %} %else %{ llvm-link -o=%t_app.bc %t.bc %t_compiler_wrappers.bc %}
// RUN: %if linux %{ llvm-link -o=%t_app.bc %t.bc %sycl_static_libs_dir/libsycl-itt-compiler-wrappers.bc %sycl_static_libs_dir/libsycl-asan.bc %} %else %{ llvm-link -o=%t_app.bc %t.bc %sycl_static_libs_dir/libsycl-itt-compiler-wrappers.bc %}

// >> ---- translate to SPIR-V
// RUN: llvm-spirv -o %t.spv %t_app.bc
// RUN: %clangxx -Wno-error=ignored-attributes %sycl_include -DSYCL_DISABLE_FALLBACK_ASSERT %cxx_std_optionc++17 %include_option %t.h %s -o %t.out %sycl_options -Xclang -verify-ignore-unexpected=note,warning %if preview-mode %{-Wno-unused-command-line-argument%}
// RUN: env SYCL_USE_KERNEL_SPV=%t.spv %{run} %t.out

// Check backward compatibility: verify that SYCL object files can be unbundled
// to extract device code as in old-offloading-model workflows.
// >> ---- unbundle compiler wrapper and asan device objects
// RUN: clang-offload-bundler -type=o -targets=sycl-spir64-unknown-unknown -input=%sycl_static_libs_dir/libsycl-itt-compiler-wrappers.old%obj_ext -output=%t_compiler_wrappers.old.bc -unbundle
// RUN: %if linux %{ clang-offload-bundler -type=o -targets=sycl-spir64-unknown-unknown -input=%sycl_static_libs_dir/libsycl-asan.old%obj_ext -output=%t_asan.old.bc -unbundle %}

// >> ---- link device code
// RUN: %if linux %{ llvm-link -o=%t_app.old.bc %t.bc %t_compiler_wrappers.old.bc %t_asan.old.bc %} %else %{ llvm-link -o=%t_app.old.bc %t.bc %t_compiler_wrappers.old.bc %}

// >> ---- translate to SPIR-V
// RUN: llvm-spirv -o %t.old.spv %t_app.old.bc
// RUN: env SYCL_USE_KERNEL_SPV=%t.old.spv %{run} %t.out

#include <iostream>
#include <sycl/detail/core.hpp>

Expand Down
Loading
Loading