[SYCL][Driver] Update devicelib link logic to stop handling deprecated options. #19592

jinge90 · 2025-07-25T06:38:26Z

We have marked "f[no-]sycl-device-lib=" option as deprecated and will remove in future major release and we also decide to reuse community "no-offloadlib" option to disable all sycl device libraries for debug purpose. This PR updates getDeviceLibraries to get rid of deprecated options and honor "no-offloadlib" while keeping previous legacy logic to handle deprecated options since they haven't been removed.
Once users touch the deprecated option, will delegate the handling to legacy "getDeviceLibraries" function. We will remove the legacy function in next major release.

…ption Signed-off-by: jinge90 <[email protected]>

Signed-off-by: jinge90 <[email protected]>

…river

Signed-off-by: jinge90 <[email protected]>

jinge90 · 2025-08-21T07:24:51Z

Hi, @mdtoguchi
Could you help review this PR?
Thanks very much.

jinge90 · 2025-08-25T02:28:34Z

Hi, @mdtoguchi
Kind ping~~~
Thanks very much.

srividya-sundaram · 2025-08-26T05:08:02Z

clang/lib/Driver/ToolChains/SYCL.cpp

+    return LibraryList;
+  }
+
+  using SYCLDeviceLibsList = SmallVector<StringRef, 8>;


The number of elements defined exceeds its declared inline capacity for SYCLDeviceLibs (more than 8 elements)
We could omit the number of inlined elements N, and just have SmallVector<StringRef>

Done.
Thanks very much.

srividya-sundaram · 2025-08-26T05:18:59Z

clang/lib/Driver/ToolChains/SYCL.cpp

+    LibSuffix = IsWindowsMSVCEnv ? ".new.obj" : ".new.o";
+  auto addLibraries = [&](const SYCLDeviceLibsList &LibsList) {
+    for (const StringRef &Lib : LibsList) {
+      SmallString<128> LibName(Lib);


Creating a new String and copying data can have performance implications, especially in loops.
Any reason to have SmallVector<StringRef> instead of SmallVector<String> ?

srividya-sundaram · 2025-08-26T05:23:08Z

clang/test/Driver/sycl-device-lib-amdgcn.cpp

-// RUN: %clangxx -### -nogpulib -fno-sycl-libspirv --sysroot=%S/Inputs/SYCL \
-// RUN: -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx906 %s 2>&1 \
+// RUN: %clangxx -### -fsycl -fsycl-targets=amdgcn-amd-amdhsa -fno-sycl-libspirv --sysroot=%S/Inputs/SYCL \
+// RUN: -Xsycl-target-backend --offload-arch=gfx908 --rocm-path=%S/Inputs/rocm %s 2>&1 \


Why is --offload-arch value changed from gfx906 to gfx908 and -nogpulib removed?

Hi, @srividya-sundaram
Previously, the test didn't add --rocm-path, so the test would run fail on machines without AMD toolchain and reported some amd device rtl files were missing. To workaround this, "nogpulib" was added and this option wouldn't impact sycl device libraries link at that time.
Now, we reuse community "--no-offloadlib" option to disable all sycl device libraries and "nogpulib" is an alias for "--no-offloadlib", so we must remove this option otherwise all sycl device libraries will be disabled and the test will fail.
After removing "nogpulib", we need to specify --rocm-path to indicate where to find the required amd device rtl files and current syclos only have dummy amd device rtl files for gfx908, so we have to change offload-arch to gfx908 otherwise the test still reported error that amd device rtl files are missing.
Thanks very much.

srividya-sundaram · 2025-08-26T05:25:52Z

clang/lib/Driver/ToolChains/SYCL.cpp

 }

+SmallVector<std::string, 8>
+SYCL::getDeviceLibraries(const Compilation &C, const llvm::Triple &TargetTriple,


Please add a quick note/comment about what this function does and a description of the edge cases if applicable.

Done.
Thanks very much.

srividya-sundaram · 2025-08-26T05:31:42Z

clang/lib/Driver/ToolChains/SYCL.cpp

+// TODO: remove getDeviceLibrariesLegacy when we remove deprecated options
+// related to sycl device library link.
+static SmallVector<std::string, 8>
+getDeviceLibrariesLegacy(const Compilation &C, const llvm::Triple &TargetTriple,


Please add a comment describing what this functions does and why it is needed.

Done.
Thanks very much.

Signed-off-by: jinge90 <[email protected]>

…river

jinge90 · 2025-08-27T03:17:50Z

Hi, @intel/llvm-gatekeepers
The pre-ci failure should be a flaky issue:
Unexpectedly Passed Tests (1):
SYCL :: Matrix/SG32/joint_matrix_bfloat16.cpp
Could you help take a look?
Thanks very much.

steffenlarsen · 2025-08-27T05:02:19Z

Looks like you were just unlucky and hit the driver uplift from #19880 but the test changes didn't get through. Should be good to go.

jinge90 requested a review from a team as a code owner July 25, 2025 06:38

jinge90 marked this pull request as draft July 25, 2025 06:38

jinge90 had a problem deploying to WindowsCILock July 25, 2025 06:38 — with GitHub Actions Failure

[SYCL][Driver] Update devicelib link logic to get rid of deprecated o…

04ec90b

…ption Signed-off-by: jinge90 <[email protected]>

jinge90 had a problem deploying to WindowsCILock July 25, 2025 07:08 — with GitHub Actions Failure

jinge90 had a problem deploying to WindowsCILock July 25, 2025 08:34 — with GitHub Actions Failure

link all sycl device libraries by default

b1e3af2

Signed-off-by: jinge90 <[email protected]>

jinge90 temporarily deployed to WindowsCILock July 25, 2025 09:05 — with GitHub Actions Inactive

Merge remote-tracking branch 'upstream/sycl' into new_add_devicelib_d…

bd9cafd

…river

jinge90 had a problem deploying to WindowsCILock August 12, 2025 08:44 — with GitHub Actions Failure

jinge90 temporarily deployed to WindowsCILock August 12, 2025 09:13 — with GitHub Actions Inactive

jinge90 had a problem deploying to WindowsCILock August 14, 2025 05:29 — with GitHub Actions Failure

Merge remote-tracking branch 'upstream/sycl' into new_add_devicelib_d…

8a35890

…river

jinge90 temporarily deployed to WindowsCILock August 14, 2025 06:04 — with GitHub Actions Inactive

Merge remote-tracking branch 'upstream/sycl' into new_add_devicelib_d…

d882eae

…river

jinge90 had a problem deploying to WindowsCILock August 15, 2025 05:51 — with GitHub Actions Error

jinge90 had a problem deploying to WindowsCILock August 15, 2025 06:17 — with GitHub Actions Error

Always link nativecpu utils and link itt according to options

636ec9d

Signed-off-by: jinge90 <[email protected]>

jinge90 had a problem deploying to WindowsCILock August 15, 2025 06:31 — with GitHub Actions Failure

jinge90 added 2 commits August 15, 2025 14:46

Select bfloat16 devicelib

f0ab62d

Signed-off-by: jinge90 <[email protected]>

make legacy API as static

ee5bcac

Signed-off-by: jinge90 <[email protected]>

jinge90 temporarily deployed to WindowsCILock August 15, 2025 07:16 — with GitHub Actions Inactive

jinge90 had a problem deploying to WindowsCILock August 21, 2025 01:52 — with GitHub Actions Failure

add device sanitizer libs

ed8c67b

Signed-off-by: jinge90 <[email protected]>

jinge90 temporarily deployed to WindowsCILock August 21, 2025 02:42 — with GitHub Actions Inactive

jinge90 temporarily deployed to WindowsCILock August 21, 2025 06:43 — with GitHub Actions Inactive

jinge90 marked this pull request as ready for review August 21, 2025 07:16

jinge90 requested a review from mdtoguchi August 21, 2025 07:16

srividya-sundaram reviewed Aug 26, 2025

View reviewed changes

jinge90 had a problem deploying to WindowsCILock August 26, 2025 07:43 — with GitHub Actions Error

jinge90 had a problem deploying to WindowsCILock August 26, 2025 07:50 — with GitHub Actions Failure

jinge90 added 2 commits August 26, 2025 16:12

Remove unnecessary string copy

8d49f7e

Signed-off-by: jinge90 <[email protected]>

remove redundant string create

a79ed5d

Signed-off-by: jinge90 <[email protected]>

jinge90 had a problem deploying to WindowsCILock August 26, 2025 08:32 — with GitHub Actions Failure

jinge90 temporarily deployed to WindowsCILock August 26, 2025 08:32 — with GitHub Actions Inactive

jinge90 temporarily deployed to WindowsCILock August 26, 2025 08:41 — with GitHub Actions Inactive

jinge90 had a problem deploying to WindowsCILock August 26, 2025 09:11 — with GitHub Actions Failure

jinge90 temporarily deployed to WindowsCILock August 26, 2025 09:11 — with GitHub Actions Inactive

jinge90 had a problem deploying to WindowsCILock August 26, 2025 13:57 — with GitHub Actions Failure

srividya-sundaram approved these changes Aug 26, 2025

View reviewed changes

jinge90 temporarily deployed to WindowsCILock August 27, 2025 01:56 — with GitHub Actions Inactive

Merge remote-tracking branch 'upstream/sycl' into new_add_devicelib_d…

005d5ab

…river

jinge90 temporarily deployed to WindowsCILock August 27, 2025 02:27 — with GitHub Actions Inactive

jinge90 requested a review from a team August 27, 2025 03:16

steffenlarsen merged commit 403ee01 into intel:sycl Aug 27, 2025
40 of 44 checks passed

steffenlarsen mentioned this pull request Aug 27, 2025

[GHA] Uplift Linux GPU RT version to 25.31.34666.3 #19880

Merged

Uh oh!

[SYCL][Driver] Update devicelib link logic to stop handling deprecated options. #19592

[SYCL][Driver] Update devicelib link logic to stop handling deprecated options. #19592

Uh oh!

Conversation

jinge90 commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jinge90 commented Aug 21, 2025

Uh oh!

jinge90 commented Aug 25, 2025

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!

jinge90 commented Aug 27, 2025

Uh oh!

steffenlarsen commented Aug 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jinge90 commented Jul 25, 2025 •

edited

Loading