-
Notifications
You must be signed in to change notification settings - Fork 798
[Driver][SYCL] Improve way sycld is pulled in for Linux based Windows… #6974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… driver Update the way sycld is pulled in by matching if msvcrtd is brought in via the command line. Cmake uses '-Xclang --dependent-lib=msvcrtd' when there is the desire to link in with the debug libraries. Check for this and use an equivalent '--dependent-lib=sycld' to pull in the SYCL runtime library. This is only done for the Linux based drivers on Windows. The MSVC based driver behavior already pulls in the proper libraries given /MDd.
|
Does SYCL library on Windows for open-source compiler include version in its name? |
@densamoilov, it was introduced here: #6745 |
clang/lib/Driver/ToolChains/MSVC.cpp
Outdated
| CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION "d.lib"); | ||
| else | ||
| CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION ".lib"); | ||
| CmdArgs.push_back("-defaultlib:sycl" SYCL_MAJOR_VERSION ".lib"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to skip this when --dependent-lib=sycld is set.
I've tried the patch with a Debug build using the CMake module and it crashed with debug mismatches. It seems that --dependent-lib is handled by clang and -defaultlib is passed to link.exe so it ends up still picking up the release SYCL library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same can be said about the addition of -defaultlib:msvcrt, as it would override the user setting as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but in the case of the CMake module it uses -nostdlib before passing -Xclang --dependent-lib=msvcrtd so -defaultlib:msvcrt doesn't get added in the toolchain.
The SYCL library currently doesn't respect -nostdlib (see: #6699 ), so if we implicitly add --dependent-lib=sycld we also need to avoid adding -defaultlib:sycld.
|
Failures are not related to my change. @intel/llvm-gatekeepers, ok for merge? |
… driver
Update the way sycld is pulled in by matching if msvcrtd is brought in via the command line. Cmake uses '-Xclang --dependent-lib=msvcrtd' when there is the desire to link in with the debug libraries. Check for this and use an equivalent '--dependent-lib=sycld' to pull in the SYCL runtime library.
This is only done for the Linux based drivers on Windows. The MSVC based driver behavior already pulls in the proper libraries given /MDd.