Repository navigation
Conversation
The linker-wrapper job located all SYCL device libraries by probing for libsycl-crt.bc and assuming every other library lived alongside it. That library isn't one of the NVPTX/AMDGCN/NativeCPU libraries actually passed, and missing libraries were silently passed with a bogus path. Add SYCLInstallationDetector::findSYCLDeviceLib() to search the device library locations for a given library, and use it in SYCLToolChain::getDeviceLibs and the sycl-post-link bfloat16 lookup. --bitcode-library= paths now come from getDeviceLibs, which diagnoses missing libraries. -sycl-device-library-location= is derived from the library the wrapper consumes (NativeCPU utils or native bfloat16). Update sycl-bc-device-libraries.cpp to use -ccc-install-dir so the test is independent of the build tree, and add a missing-library check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
iclsrc
left a comment
iclsrc
left a comment
There was a problem hiding this comment.
This PR adds SYCLInstallationDetector::findSYCLDeviceLib and uses it in place of several hand-rolled search loops. In the linker wrapper, --bitcode-library paths now come from getDeviceLibs, so they are full resolved paths. -sycl-device-library-location is now derived from where a representative library (nativecpu_utils or native-bfloat16) is found. The refactor looks correct and removes duplication. I found no silent-miscompilation or SYCL pass-invariant issues, because the change is driver-only. One test gap and one behavior note are below.
| DeviceLibPaths.emplace_back(Path.str()); | ||
| } | ||
|
|
||
| std::optional<SmallString<128>> |
There was a problem hiding this comment.
🔵 Suggestion: findSYCLDeviceLib now uses D.getVFS().exists(). The code it replaces used llvm::sys::fs::exists. This is arguably better, since it honors -ivfsoverlay. It is still a behavior change for all callers, so it is worth mentioning in the PR description. The header also uses std::optional and should include <optional> explicitly, to avoid relying on transitive includes.
|
@intel/llvm-reviewers-runtime , @dm-vodopyanov , Can somebody take a look? I added a couple of new files to the LIT test location, and for some reason github added llvm-reviewers-runtime. I checked CODEOWNERS and there is no indication why. |
|
@srividya-sundaram, any additional comments on this? |
The linker-wrapper job located all SYCL device libraries by probing for libsycl-crt.bc and assuming every other library lived alongside it. That library isn't one of the NVPTX/AMDGCN/NativeCPU libraries actually passed, and missing libraries were silently passed with a bogus path.
Add SYCLInstallationDetector::findSYCLDeviceLib() to search the device library locations for a given library, and use it in SYCLToolChain::getDeviceLibs and the sycl-post-link bfloat16 lookup. --bitcode-library= paths now come from getDeviceLibs, which diagnoses missing libraries. -sycl-device-library-location= is derived from the library the wrapper consumes (NativeCPU utils or native bfloat16).
Update sycl-bc-device-libraries.cpp to use -ccc-install-dir so the test is independent of the build tree, and add a missing-library check.