Skip to content

Conversation

@YixingZhang007
Copy link
Contributor

@YixingZhang007 YixingZhang007 commented Oct 30, 2025

This pull request refactors the SYCL device library build and end-to-end tests to support the new offloading model, while maintaining backward compatibility for the old offloading model, with the following key changes.

  1. The default device library (such as libsycl-itt-compiler-wrappers.o) is now compiled using the new offloading model, and the library ending with .new.o (such as libsycl-itt-compiler-wrappers.new.o) has been renamed to .old.o (such as libsycl-itt-compiler-wrappers.old.o), representing builds with the old offloading model. These changes are made in libdevice/cmake/modules/SYCLLibdevice.cmake.
  2. The end-to-end tests in sycl/test-e2e/Config/kernel_from_file.cpp and sycl/test-e2e/SeparateCompile/test.cpp have been updated. The tests now validate both the new offloading model, and the backward compatibility for the old model (using clang-offload-bundler to extract device code from libraries).

@YixingZhang007 YixingZhang007 requested review from a team as code owners October 30, 2025 04:33
@YixingZhang007 YixingZhang007 marked this pull request as draft October 30, 2025 04:33
@YixingZhang007 YixingZhang007 changed the title [SYCL][NewOffloadModel] Remove Clang Offload Bundler testing to fix E2E test failures with new offloading model [SYCL][NewOffloadModel] Remove Clang Offload Bundler testing to fix E2E test failures Oct 30, 2025
@YixingZhang007 YixingZhang007 changed the title [SYCL][NewOffloadModel] Remove Clang Offload Bundler testing to fix E2E test failures [SYCL][NewOffloadModel] Remove Clang Offload Bundler testing for new offloading model Oct 30, 2025
@YixingZhang007 YixingZhang007 changed the title [SYCL][NewOffloadModel] Remove Clang Offload Bundler testing for new offloading model [SYCL][NewOffloadModel] Update SYCLLibdevice generation for new offloading model Nov 2, 2025
@YixingZhang007 YixingZhang007 changed the title [SYCL][NewOffloadModel] Update SYCLLibdevice generation for new offloading model [SYCL][NewOffloadModel] Update SYCL device library generation for new offloading model Nov 2, 2025
@YixingZhang007
Copy link
Contributor Author

The modifications to set the new offloading model as default in this PR were directly copied from (#15121). These changes will be removed once CI confirms there are no regressions.

@YixingZhang007 YixingZhang007 marked this pull request as ready for review November 2, 2025 20:31
@YixingZhang007 YixingZhang007 marked this pull request as draft November 2, 2025 23:32
Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

thanks for the PR! one higher level comment below

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

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants