Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[clang] [Driver] Ensure we error on lto with link.exe and target *-windows-msvc on non cl driver modes #109607

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[clang] [Driver] Ensure we error on lto with link.exe and target *-windows-msvc on non cl driver modes #109607

wants to merge 1 commit into from
+8 −5

Conversation

MaxEW707
Copy link
Contributor

Similar to previous PRs I've done to change some IsCLMode checks to isWindowsMSVCEnvironment.
I stumbled into this one accidentally last week. I did some greps and I think this is the last one for now. All the remaining IsCLMode checks are only valid for the cl driver mode.

For the *-windows-msvc target MSVC link.exe and lld are supported. LTO isn't supported with MSVC link.exe and so we error when lto is enabled but MSVC link.exe is being used. However we only error if the driver mode is cl.
If we are using the clang driver with the *-windows-msvc target then ensure an error is also emitted when LTO and MSVC link.exe are used together.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Sep 23, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 23, 2024

@llvm/pr-subscribers-clang-driver

Author: Max Winkler (MaxEW707)

Changes

Similar to previous PRs I've done to change some IsCLMode checks to isWindowsMSVCEnvironment.
I stumbled into this one accidentally last week. I did some greps and I think this is the last one for now. All the remaining IsCLMode checks are only valid for the cl driver mode.

For the *-windows-msvc target MSVC link.exe and lld are supported. LTO isn't supported with MSVC link.exe and so we error when lto is enabled but MSVC link.exe is being used. However we only error if the driver mode is cl.
If we are using the clang driver with the *-windows-msvc target then ensure an error is also emitted when LTO and MSVC link.exe are used together.

Full diff: https://github.com/llvm/llvm-project/pull/109607.diff

6 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+2-1)
  • (modified) clang/test/Driver/cl-options.c (+2)
  • (modified) clang/test/Driver/clang_f_opts.c (+1-1)
  • (modified) clang/test/Driver/lto.c (+1-1)
  • (modified) clang/test/Driver/thinlto.c (+1-1)
  • (modified) clang/test/Driver/thinlto.cu (+1-1)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 44548fa9d706fb..a9506cf911b950 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4011,7 +4011,8 @@ void Driver::handleArguments(Compilation &C, DerivedArgList &Args,
     // Emitting LLVM while linking disabled except in HIPAMD Toolchain
     if (Args.hasArg(options::OPT_emit_llvm) && !Args.hasArg(options::OPT_hip_link))
       Diag(clang::diag::err_drv_emit_llvm_link);
-    if (IsCLMode() && LTOMode != LTOK_None &&
+    if (C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment() &&
+        LTOMode != LTOK_None &&
         !Args.getLastArgValue(options::OPT_fuse_ld_EQ)
              .equals_insensitive("lld"))
       Diag(clang::diag::err_drv_lto_without_lld);
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index a6f338533ad763..f0de6289d95223 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -626,6 +626,8 @@
 // LTO-THIN: -flto=thin
 
 // RUN: not %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
+// RUN: not %clang_cl -### -fuse-ld=link -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
+// RUN: not %clang -### --target=x86_64-windows-msvc -fuse-ld=link -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
 // LTO-WITHOUT-LLD: LTO requires -fuse-ld=lld
 
 // RUN: %clang_cl  -### -- %s 2>&1 | FileCheck -check-prefix=NOCFGUARD %s
diff --git a/clang/test/Driver/clang_f_opts.c b/clang/test/Driver/clang_f_opts.c
index adb6f075b6c15d..5dfd86f7515236 100644
--- a/clang/test/Driver/clang_f_opts.c
+++ b/clang/test/Driver/clang_f_opts.c
@@ -605,7 +605,7 @@
 // RUN: %clang -### -S -fjmc -g -fno-jmc -target x86_64-pc-windows-msvc %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC %s
 // RUN: %clang -### -S -fjmc -g -target x86_64-unknown-linux %s 2>&1 | FileCheck -check-prefix=CHECK_JMC %s
 // RUN: %clang -### -S -fjmc -g -fno-jmc -target x86_64-unknown-linux %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC %s
-// RUN: %clang -### -fjmc -g -flto -target x86_64-pc-windows-msvc %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC_LTO %s
+// RUN: %clang -### -fjmc -g -flto -fuse-ld=lld -target x86_64-pc-windows-msvc %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC_LTO %s
 // RUN: %clang -### -fjmc -g -flto -target x86_64-unknown-linux %s 2>&1 | FileCheck -check-prefix=CHECK_JMC_LTO %s
 // RUN: %clang -### -fjmc -g -flto -fno-jmc -target x86_64-unknown-linux %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC_LTO %s
 // CHECK_JMC_WARN: -fjmc requires debug info. Use -g or debug options that enable debugger's stepping function; option ignored
diff --git a/clang/test/Driver/lto.c b/clang/test/Driver/lto.c
index b6c89eb99e2741..d3e22d9a1196b2 100644
--- a/clang/test/Driver/lto.c
+++ b/clang/test/Driver/lto.c
@@ -5,7 +5,7 @@
 // CHECK-COMPILE-ACTIONS: 2: compiler, {1}, ir
 // CHECK-COMPILE-ACTIONS: 3: backend, {2}, lto-bc
 
-// RUN: %clang -ccc-print-phases %s -flto 2> %t
+// RUN: %clang -ccc-print-phases -fuse-ld=lld %s -flto 2> %t
 // RUN: FileCheck -check-prefix=CHECK-COMPILELINK-ACTIONS < %t %s
 //
 // CHECK-COMPILELINK-ACTIONS: 0: input, "{{.*}}lto.c", c
diff --git a/clang/test/Driver/thinlto.c b/clang/test/Driver/thinlto.c
index e08435f71cc0b0..b54ef307628851 100644
--- a/clang/test/Driver/thinlto.c
+++ b/clang/test/Driver/thinlto.c
@@ -5,7 +5,7 @@
 // CHECK-COMPILE-ACTIONS: 2: compiler, {1}, ir
 // CHECK-COMPILE-ACTIONS: 3: backend, {2}, lto-bc
 
-// RUN: %clang -ccc-print-phases %s -flto=thin 2> %t
+// RUN: %clang -fuse-ld=lld -ccc-print-phases %s -flto=thin 2> %t
 // RUN: FileCheck -check-prefix=CHECK-COMPILELINK-ACTIONS < %t %s
 //
 // CHECK-COMPILELINK-ACTIONS: 0: input, "{{.*}}thinlto.c", c
diff --git a/clang/test/Driver/thinlto.cu b/clang/test/Driver/thinlto.cu
index c175aae37c718d..b7875884a24aa8 100644
--- a/clang/test/Driver/thinlto.cu
+++ b/clang/test/Driver/thinlto.cu
@@ -6,7 +6,7 @@
 // CHECK-COMPILE-ACTIONS-NOT: lto-bc
 // CHECK-COMPILE-ACTIONS: 12: backend, {11}, lto-bc, (host-cuda)
 
-// RUN: %clangxx -ccc-print-phases --no-offload-new-driver -nocudainc -nocudalib %s -flto=thin 2> %t
+// RUN: %clangxx -ccc-print-phases -fuse-ld=lld --no-offload-new-driver -nocudainc -nocudalib %s -flto=thin 2> %t
 // RUN: FileCheck -check-prefix=CHECK-COMPILELINK-ACTIONS < %t %s
 //
 // CHECK-COMPILELINK-ACTIONS: 0: input, "{{.*}}thinlto.cu", cuda, (host-cuda)

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 23, 2024

@llvm/pr-subscribers-clang

Author: Max Winkler (MaxEW707)

Changes

Similar to previous PRs I've done to change some IsCLMode checks to isWindowsMSVCEnvironment.
I stumbled into this one accidentally last week. I did some greps and I think this is the last one for now. All the remaining IsCLMode checks are only valid for the cl driver mode.

For the *-windows-msvc target MSVC link.exe and lld are supported. LTO isn't supported with MSVC link.exe and so we error when lto is enabled but MSVC link.exe is being used. However we only error if the driver mode is cl.
If we are using the clang driver with the *-windows-msvc target then ensure an error is also emitted when LTO and MSVC link.exe are used together.

Full diff: https://github.com/llvm/llvm-project/pull/109607.diff

6 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+2-1)
  • (modified) clang/test/Driver/cl-options.c (+2)
  • (modified) clang/test/Driver/clang_f_opts.c (+1-1)
  • (modified) clang/test/Driver/lto.c (+1-1)
  • (modified) clang/test/Driver/thinlto.c (+1-1)
  • (modified) clang/test/Driver/thinlto.cu (+1-1)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 44548fa9d706fb..a9506cf911b950 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4011,7 +4011,8 @@ void Driver::handleArguments(Compilation &C, DerivedArgList &Args,
     // Emitting LLVM while linking disabled except in HIPAMD Toolchain
     if (Args.hasArg(options::OPT_emit_llvm) && !Args.hasArg(options::OPT_hip_link))
       Diag(clang::diag::err_drv_emit_llvm_link);
-    if (IsCLMode() && LTOMode != LTOK_None &&
+    if (C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment() &&
+        LTOMode != LTOK_None &&
         !Args.getLastArgValue(options::OPT_fuse_ld_EQ)
              .equals_insensitive("lld"))
       Diag(clang::diag::err_drv_lto_without_lld);
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index a6f338533ad763..f0de6289d95223 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -626,6 +626,8 @@
 // LTO-THIN: -flto=thin
 
 // RUN: not %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
+// RUN: not %clang_cl -### -fuse-ld=link -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
+// RUN: not %clang -### --target=x86_64-windows-msvc -fuse-ld=link -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
 // LTO-WITHOUT-LLD: LTO requires -fuse-ld=lld
 
 // RUN: %clang_cl  -### -- %s 2>&1 | FileCheck -check-prefix=NOCFGUARD %s
diff --git a/clang/test/Driver/clang_f_opts.c b/clang/test/Driver/clang_f_opts.c
index adb6f075b6c15d..5dfd86f7515236 100644
--- a/clang/test/Driver/clang_f_opts.c
+++ b/clang/test/Driver/clang_f_opts.c
@@ -605,7 +605,7 @@
 // RUN: %clang -### -S -fjmc -g -fno-jmc -target x86_64-pc-windows-msvc %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC %s
 // RUN: %clang -### -S -fjmc -g -target x86_64-unknown-linux %s 2>&1 | FileCheck -check-prefix=CHECK_JMC %s
 // RUN: %clang -### -S -fjmc -g -fno-jmc -target x86_64-unknown-linux %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC %s
-// RUN: %clang -### -fjmc -g -flto -target x86_64-pc-windows-msvc %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC_LTO %s
+// RUN: %clang -### -fjmc -g -flto -fuse-ld=lld -target x86_64-pc-windows-msvc %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC_LTO %s
 // RUN: %clang -### -fjmc -g -flto -target x86_64-unknown-linux %s 2>&1 | FileCheck -check-prefix=CHECK_JMC_LTO %s
 // RUN: %clang -### -fjmc -g -flto -fno-jmc -target x86_64-unknown-linux %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC_LTO %s
 // CHECK_JMC_WARN: -fjmc requires debug info. Use -g or debug options that enable debugger's stepping function; option ignored
diff --git a/clang/test/Driver/lto.c b/clang/test/Driver/lto.c
index b6c89eb99e2741..d3e22d9a1196b2 100644
--- a/clang/test/Driver/lto.c
+++ b/clang/test/Driver/lto.c
@@ -5,7 +5,7 @@
 // CHECK-COMPILE-ACTIONS: 2: compiler, {1}, ir
 // CHECK-COMPILE-ACTIONS: 3: backend, {2}, lto-bc
 
-// RUN: %clang -ccc-print-phases %s -flto 2> %t
+// RUN: %clang -ccc-print-phases -fuse-ld=lld %s -flto 2> %t
 // RUN: FileCheck -check-prefix=CHECK-COMPILELINK-ACTIONS < %t %s
 //
 // CHECK-COMPILELINK-ACTIONS: 0: input, "{{.*}}lto.c", c
diff --git a/clang/test/Driver/thinlto.c b/clang/test/Driver/thinlto.c
index e08435f71cc0b0..b54ef307628851 100644
--- a/clang/test/Driver/thinlto.c
+++ b/clang/test/Driver/thinlto.c
@@ -5,7 +5,7 @@
 // CHECK-COMPILE-ACTIONS: 2: compiler, {1}, ir
 // CHECK-COMPILE-ACTIONS: 3: backend, {2}, lto-bc
 
-// RUN: %clang -ccc-print-phases %s -flto=thin 2> %t
+// RUN: %clang -fuse-ld=lld -ccc-print-phases %s -flto=thin 2> %t
 // RUN: FileCheck -check-prefix=CHECK-COMPILELINK-ACTIONS < %t %s
 //
 // CHECK-COMPILELINK-ACTIONS: 0: input, "{{.*}}thinlto.c", c
diff --git a/clang/test/Driver/thinlto.cu b/clang/test/Driver/thinlto.cu
index c175aae37c718d..b7875884a24aa8 100644
--- a/clang/test/Driver/thinlto.cu
+++ b/clang/test/Driver/thinlto.cu
@@ -6,7 +6,7 @@
 // CHECK-COMPILE-ACTIONS-NOT: lto-bc
 // CHECK-COMPILE-ACTIONS: 12: backend, {11}, lto-bc, (host-cuda)
 
-// RUN: %clangxx -ccc-print-phases --no-offload-new-driver -nocudainc -nocudalib %s -flto=thin 2> %t
+// RUN: %clangxx -ccc-print-phases -fuse-ld=lld --no-offload-new-driver -nocudainc -nocudalib %s -flto=thin 2> %t
 // RUN: FileCheck -check-prefix=CHECK-COMPILELINK-ACTIONS < %t %s
 //
 // CHECK-COMPILELINK-ACTIONS: 0: input, "{{.*}}thinlto.cu", cuda, (host-cuda)

MaskRay
MaskRay reviewed Sep 23, 2024
View reviewed changes
clang/test/Driver/clang_f_opts.c
@@ -605,7 +605,7 @@
// RUN: %clang -### -S -fjmc -g -fno-jmc -target x86_64-pc-windows-msvc %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC %s
// RUN: %clang -### -S -fjmc -g -target x86_64-unknown-linux %s 2>&1 | FileCheck -check-prefix=CHECK_JMC %s
// RUN: %clang -### -S -fjmc -g -fno-jmc -target x86_64-unknown-linux %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC %s
// RUN: %clang -### -fjmc -g -flto -target x86_64-pc-windows-msvc %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC_LTO %s
// RUN: %clang -### -fjmc -g -flto -fuse-ld=lld -target x86_64-pc-windows-msvc %s 2>&1 | FileCheck -check-prefix=CHECK_NOJMC_LTO %s
Copy link
Member

Choose a reason for hiding this comment

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

-fuse-ld=lld would lead to a failure if lld is not found in program paths. This failure happens on Linux, not sure about Windows.

MaskRay
MaskRay reviewed Sep 23, 2024
View reviewed changes
clang/test/Driver/lto.c
@@ -5,7 +5,7 @@
// CHECK-COMPILE-ACTIONS: 2: compiler, {1}, ir
// CHECK-COMPILE-ACTIONS: 3: backend, {2}, lto-bc

// RUN: %clang -ccc-print-phases %s -flto 2> %t
// RUN: %clang -ccc-print-phases -fuse-ld=lld %s -flto 2> %t
Copy link
Member

@MaskRay MaskRay Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This requirement is really non-obvious. The default target triple could be windows-msvc and without -fuse-ld=lld there would be an error?

I don't think this is reasonable for other people that write -ccc-print-phases tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaskRay MaskRay MaskRay left review comments

@nico nico Awaiting requested review from nico

@zmodem zmodem Awaiting requested review from zmodem

Assignees
No one assigned
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MaxEW707 @llvmbot @MaskRay