Skip to content

[Driver][SYCL] Update sycl lib linking with -fms-runtime-lib #19380

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

Merged
merged 1 commit into from
Jul 16, 2025

Conversation

mdtoguchi
Copy link
Contributor

The usage of -fms-runtime-lib=val will pull in the default libraries for MSVC linking as well as the dependent SYCL libraries when using the Linux based driver with the MSVC target triple. There are special interactions with the --dependent-lib=msvcrtd library we have that control the inclusion of sycl.lib to prevent linktime problems.

Extend these behaviors to usage of -fms-runtime-lib=dll_dbg, preventing duplicate instances of the sycl.lib in the --dependent-lib usage as well as when performing the final link.

The usage of `-fms-runtime-lib=val` will pull in the default libraries
for MSVC linking as well as the dependent SYCL libraries when using the
Linux based driver with the MSVC target triple.  There are special
interactions with the --dependent-lib=msvcrtd library we have that
control the inclusion of sycl.lib to prevent linktime problems.

Extend these behaviors to usage of `-fms-runtime-lib=dll_dbg`,
preventing duplicate instances of the sycl.lib in the --dependent-lib
usage as well as when performing the final link.
@mdtoguchi mdtoguchi requested a review from a team as a code owner July 9, 2025 22:53
@mdtoguchi
Copy link
Contributor Author

@intel/dpcpp-clang-driver-reviewers please take a look.

// equivalent. Do not add the -defaultlib as it conflicts.
if (!isDependentLibAdded(Args, "msvcrtd")) {
// When msvcrtd is added via --dependent-lib or -fms-runtime-lib=dll_dbg we
// add the sycld equivalent. Do not add the -defaultlib as it conflicts.
Copy link
Contributor

Choose a reason for hiding this comment

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

*sycl ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reference here should be sycld as given the conditions the sycld library will already have been added via different means, so we do not want to add the sycl variant as it will conflict.

@mdtoguchi
Copy link
Contributor Author

@srividya-sundaram, any other review comments?

@srividya-sundaram
Copy link
Contributor

Were you able to view my past comments?

What is the behavior if --offload-new-driver is not passed?
Is this supported in clang_cl mode?

I was waiting for your response.

@mdtoguchi
Copy link
Contributor Author

mdtoguchi commented Jul 16, 2025

Were you able to view my past comments?

What is the behavior if --offload-new-driver is not passed? Is this supported in clang_cl mode?

I was waiting for your response.

I don't see any other comments other than your *sycl comment. But to try and answer your comments here, --offload-new-driver does not impact the behavior, this is specific to host linking. clang_cl is not impacted here either, as all the behaviors are for non-CLMode.

// equivalent. Do not add the -defaultlib as it conflicts.
if (!isDependentLibAdded(Args, "msvcrtd")) {
// When msvcrtd is added via --dependent-lib or -fms-runtime-lib=dll_dbg we
// add the sycld equivalent. Do not add the -defaultlib as it conflicts.
Copy link
Contributor

Choose a reason for hiding this comment

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

// CHECK-LINK-SYCL-DEBUG: "--dependent-lib=sycl{{[0-9]*}}d"
// CHECK-LINK-SYCL-DEBUG-NOT: "-defaultlib:sycl{{[0-9]*}}.lib"

/// Only a single instance of sycld should be pulled in when both the
/// -Xclang --dependent-lib=msvcrtd and -fms-runtime-lib=dll_dbg is used.
// RUN: %clangxx -fsycl --offload-new-driver -fms-runtime-lib=dll_dbg -Xclang \
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supported in clang_cl mode?

@@ -292,9 +292,21 @@
// RUN: %clangxx -fsycl --offload-new-driver -Xclang --dependent-lib=msvcrtd \
// RUN: -target x86_64-unknown-windows-msvc -### %s 2>&1 \
// RUN: | FileCheck -check-prefix=CHECK-LINK-SYCL-DEBUG %s
/// Check sycld.lib is pulled in with -fms-runtime-lib=dll_dbg
// RUN: %clangxx -fsycl --offload-new-driver -fms-runtime-lib=dll_dbg \
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the behavior if --offload-new-driver is not passed?

@mdtoguchi
Copy link
Contributor Author

@intel/llvm-gatekeepers, this is ready for merge - thanks!

@sarnex sarnex merged commit 7e1ea22 into intel:sycl Jul 16, 2025
25 checks passed
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.

4 participants