Skip to content

Conversation

jzc
Copy link
Contributor

@jzc jzc commented Oct 9, 2025

This PR adds the -sycl-bc-device-libraries option to clang-linker-wrapper properly link bitcode device libraries that are used in NVIDIA and AMD GPU targets in SYCL offloading.

SYCL::getDeviceLibraries() returns both fat objects and bitcode device libraries mixed together, while the -sycl-device-libraries option only handles fat objects. This was causing clang-linker-wrapper to not link the libdevice for nvptx/AMD targets.

This PR also removes the -sycl-nvptx-device-libraries option, which seemed to be doing the same thing but only for nvptx targets.

@jzc jzc requested review from a team as code owners October 9, 2025 17:51
@bader
Copy link
Contributor

bader commented Oct 9, 2025

This PR adds the -sycl-bc-device-libraries option to clang-linker-wrapper properly link bitcode device libraries that are used in NVIDIA and AMD GPU targets in SYCL offloading.

SYCL::getDeviceLibraries() returns both fat objects and bitcode device libraries mixed together, while the -sycl-device-libraries option only handles fat objects. This was causing clang-linker-wrapper to not link the libdevice for nvptx/AMD targets.

This PR also removes the -sycl-nvptx-device-libraries option, which seemed to be doing the same thing but only for nvptx targets.

This seems unnecessary. clang-linker-wrapper already supports bitcode-library and builtin-bitcode options which driver can use to pass libdevice.

@jzc
Copy link
Contributor Author

jzc commented Oct 9, 2025

@bader I did try using these options, but I wasn't able to get them to work, although I'm unsure the reason. Let me try again see what went wrong and see if I can use these options

@jzc jzc temporarily deployed to WindowsCILock October 9, 2025 18:26 — with GitHub Actions Inactive
@jzc jzc temporarily deployed to WindowsCILock October 9, 2025 18:26 — with GitHub Actions Inactive
Comment on lines +11315 to +11316
Args.hasFlag(options::OPT_fsycl_device_lib_jit_link,
options::OPT_fno_sycl_device_lib_jit_link, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: #20326 removes these options.

@bader
Copy link
Contributor

bader commented Oct 9, 2025

@bader I did try using these options, but I wasn't able to get them to work, although I'm unsure the reason. Let me try again see what went wrong and see if I can use these options

Please, pay attention to the way the driver builds command line arguments for the clang-linker-wrapper. SYCL specific option accepts a list of library paths separated by coma, whereas existing options accept only one path. To pass multiple libraries the driver must pass multiple arguments: one per library.

Copy link
Contributor

@YuriPlyakhin YuriPlyakhin left a comment

Choose a reason for hiding this comment

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

I'll review, after comments from Alexey are addressed.

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.

3 participants