[SYCL][ClangLinkerWrapper] Remove -sycl-device-libraries option#21945
[SYCL][ClangLinkerWrapper] Remove -sycl-device-libraries option#21945srividya-sundaram wants to merge 3 commits intointel:syclfrom
Conversation
Remove the -sycl-device-libraries option and all related code from clang-linker-wrapper, as it is no longer used by either old or new offload models.
| reportError(std::move(Err)); | ||
| // Gather device library files from --bitcode-library option. | ||
| const llvm::Triple Triple(Args.getLastArgValue(OPT_triple_EQ)); | ||
| SmallVector<std::string, 16> ExtractedDeviceLibFiles; |
There was a problem hiding this comment.
Only the unused device library extraction code was removed. The general fat object extraction logic remains in place for processing user input files/libraries.
What was removed was the old device-library-specific extraction path in sycl::linkDevice(), since fat object device libraries (libsycl-*.new.o) no longer exist and device libraries are now passed directly as .bc files via --bitcode-library=
|
Also updated 7 test files to use |
mdtoguchi
left a comment
mdtoguchi
left a comment
There was a problem hiding this comment.
Nice cleanup - just some comments on a test.
| def sycl_device_lib_EQ : CommaJoined<["--", "-"], "sycl-device-libraries=">, | ||
| Flags<[WrapperOnlyOption]>, | ||
| HelpText<"A comma separated list of device libraries that are linked during the device link.">; | ||
| def sycl_device_library_location_EQ : Joined<["--", "-"], |
There was a problem hiding this comment.
nit (not necessarily for this PR) - just wondering why this option is still needed and if it is an opportunity for more clean up.
There was a problem hiding this comment.
The -sycl-device-library-location= option is still used/needed by both old and new offload models.
It serves two purposes that are still actively used:
sycl-post-link bfloat16 Device Library Lookup here
Required for NativeCPU target to find utilities library here
Also the driver passes this option to clang-linker-wrapper with the device library directory path.
So we shouldn't be removing this option.
YuriPlyakhin
left a comment
YuriPlyakhin
left a comment
There was a problem hiding this comment.
thank you, very nice clean up.
LGTM - I only reviewed Clang.cpp and clang-linker-wrapper changes. Did not look in details in test changes.
This SYCL pre commit test failure is unrelated to this patch. |
3 participants
Remove the -sycl-device-libraries option and all related code from clang-linker-wrapper, as it is no longer used by either old or new offload models.