Skip to content

[LinkerWrapper] Clean up options after proper forwarding #126297

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 2 commits into from
Feb 7, 2025

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Feb 7, 2025

Summary:
Recent changes made a lot of this stuff redundant or unused, clean it up
a bit.

Also snuck in a change to pass the CUDA path since we still use it for
fatbinary internally.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Feb 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2025

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Joseph Huber (jhuber6)

Changes

Summary:
Recent changes made a lot of this stuff redundant or unused, clean it up
a bit.

Also snuck in a change to pass the CUDA path since we still use it for
fatbinary internally.


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

4 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+2)
  • (modified) clang/test/Driver/linker-wrapper.c (+12-12)
  • (modified) clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp (+1-21)
  • (modified) clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td (-14)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index c0891d46b0a62cd..0dc87814ec04a47 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -9223,6 +9223,8 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
       Args.MakeArgString("--host-triple=" + getToolChain().getTripleString()));
   if (Args.hasArg(options::OPT_v))
     CmdArgs.push_back("--wrapper-verbose");
+  if (Arg *A = Args.getLastArg(options::OPT_cuda_path_EQ))
+    CmdArgs.push_back(Args.MakeArgString(Twine("--cuda-path=") + A->getValue()));
 
   // Construct the link job so we can wrap around it.
   Linker->ConstructJob(C, JA, Output, Inputs, Args, LinkingOutput);
diff --git a/clang/test/Driver/linker-wrapper.c b/clang/test/Driver/linker-wrapper.c
index f416ee5f4463bcc..e7b7af7bdfbf32d 100644
--- a/clang/test/Driver/linker-wrapper.c
+++ b/clang/test/Driver/linker-wrapper.c
@@ -21,16 +21,16 @@ __attribute__((visibility("protected"), used)) int x;
 // RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
 // RUN:   --linker-path=/usr/bin/ld %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=NVPTX-LINK
 
-// NVPTX-LINK: clang{{.*}} -o {{.*}}.img --target=nvptx64-nvidia-cuda -march=sm_70 -O2 -flto {{.*}}.o {{.*}}.o
+// NVPTX-LINK: clang{{.*}} -o {{.*}}.img --target=nvptx64-nvidia-cuda -march=sm_70 {{.*}}.o {{.*}}.o
 
 // RUN: clang-offload-packager -o %t.out \
 // RUN:   --image=file=%t.elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70 \
 // RUN:   --image=file=%t.elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
 // RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o -fembed-offload-object=%t.out
-// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run --device-debug -O0 \
+// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run --device-compiler=-g \
 // RUN:   --linker-path=/usr/bin/ld %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=NVPTX-LINK-DEBUG
 
-// NVPTX-LINK-DEBUG: clang{{.*}} -o {{.*}}.img --target=nvptx64-nvidia-cuda -march=sm_70 -O2 -flto {{.*}}.o {{.*}}.o -g
+// NVPTX-LINK-DEBUG: clang{{.*}} --target=nvptx64-nvidia-cuda -march=sm_70 {{.*}}-g
 
 // RUN: clang-offload-packager -o %t.out \
 // RUN:   --image=file=%t.elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908 \
@@ -39,16 +39,16 @@ __attribute__((visibility("protected"), used)) int x;
 // RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
 // RUN:   --linker-path=/usr/bin/ld %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=AMDGPU-LINK
 
-// AMDGPU-LINK: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx908 -O2 -flto -Wl,--no-undefined {{.*}}.o {{.*}}.o
+// AMDGPU-LINK: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx908 -Wl,--no-undefined {{.*}}.o {{.*}}.o
 
 // RUN: clang-offload-packager -o %t.out \
 // RUN:   --image=file=%t.amdgpu.bc,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx1030 \
 // RUN:   --image=file=%t.amdgpu.bc,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx1030
 // RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o -fembed-offload-object=%t.out
-// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run --save-temps -O2 \
+// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run --device-compiler=--save-temps \
 // RUN:   --linker-path=/usr/bin/ld %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=AMDGPU-LTO-TEMPS
 
-// AMDGPU-LTO-TEMPS: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -O2 -flto -Wl,--no-undefined {{.*}}.o -save-temps
+// AMDGPU-LTO-TEMPS: clang{{.*}} --target=amdgcn-amd-amdhsa -mcpu=gfx1030 {{.*}}-save-temps
 
 // RUN: clang-offload-packager -o %t.out \
 // RUN:   --image=file=%t.elf.o,kind=openmp,triple=x86_64-unknown-linux-gnu \
@@ -59,7 +59,7 @@ __attribute__((visibility("protected"), used)) int x;
 // RUN:   --linker-path=/usr/bin/ld.lld --whole-archive %t.a --no-whole-archive \
 // RUN:   %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=CPU-LINK
 
-// CPU-LINK: clang{{.*}} -o {{.*}}.img --target=x86_64-unknown-linux-gnu -march=native -O2 -flto -Wl,--no-undefined {{.*}}.o {{.*}}.o -Wl,-Bsymbolic -shared -Wl,--whole-archive {{.*}}.a -Wl,--no-whole-archive
+// CPU-LINK: clang{{.*}} -o {{.*}}.img --target=x86_64-unknown-linux-gnu -march=native -Wl,--no-undefined {{.*}}.o {{.*}}.o -Wl,-Bsymbolic -shared -Wl,--whole-archive {{.*}}.a -Wl,--no-whole-archive
 
 // RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o
 // RUN: clang-linker-wrapper --dry-run --host-triple=x86_64-unknown-linux-gnu -mllvm -openmp-opt-disable \
@@ -148,7 +148,7 @@ __attribute__((visibility("protected"), used)) int x;
 // RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run --clang-backend \
 // RUN:   --linker-path=/usr/bin/ld %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=CLANG-BACKEND
 
-// CLANG-BACKEND: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx908 -O2 -flto -Wl,--no-undefined {{.*}}.o
+// CLANG-BACKEND: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx908 -Wl,--no-undefined {{.*}}.o
 
 // RUN: clang-offload-packager -o %t.out \
 // RUN:   --image=file=%t.elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
@@ -171,8 +171,8 @@ __attribute__((visibility("protected"), used)) int x;
 // RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
 // RUN:   --linker-path=/usr/bin/ld %t-on.o %t-off.o %t.a -o a.out 2>&1 | FileCheck %s --check-prefix=AMD-TARGET-ID
 
-// AMD-TARGET-ID: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx90a:xnack+ -O2 -flto -Wl,--no-undefined {{.*}}.o {{.*}}.o
-// AMD-TARGET-ID: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx90a:xnack- -O2 -flto -Wl,--no-undefined {{.*}}.o {{.*}}.o
+// AMD-TARGET-ID: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx90a:xnack+ -Wl,--no-undefined {{.*}}.o {{.*}}.o
+// AMD-TARGET-ID: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx90a:xnack- -Wl,--no-undefined {{.*}}.o {{.*}}.o
 
 // RUN: clang-offload-packager -o %t-lib.out \
 // RUN:   --image=file=%t.elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=generic
@@ -187,8 +187,8 @@ __attribute__((visibility("protected"), used)) int x;
 // RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
 // RUN:   --linker-path=/usr/bin/ld %t1.o %t2.o %t.a -o a.out 2>&1 | FileCheck %s --check-prefix=ARCH-ALL
 
-// ARCH-ALL: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx90a -O2 -flto -Wl,--no-undefined {{.*}}.o {{.*}}.o
-// ARCH-ALL: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx908 -O2 -flto -Wl,--no-undefined {{.*}}.o {{.*}}.o
+// ARCH-ALL: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx90a -Wl,--no-undefined {{.*}}.o {{.*}}.o
+// ARCH-ALL: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx908 -Wl,--no-undefined {{.*}}.o {{.*}}.o
 
 // RUN: clang-offload-packager -o %t.out \
 // RUN:   --image=file=%t.elf.o,kind=openmp,triple=x86_64-unknown-linux-gnu \
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index b189cfee674dd3e..4124feefc739752 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -485,7 +485,6 @@ Expected<StringRef> clang(ArrayRef<StringRef> InputFiles, const ArgList &Args) {
   if (!TempFileOrErr)
     return TempFileOrErr.takeError();
 
-  StringRef OptLevel = Args.getLastArgValue(OPT_opt_level, "O2");
   SmallVector<StringRef, 16> CmdArgs{
       *ClangPath,
       "--no-default-config",
@@ -493,12 +492,10 @@ Expected<StringRef> clang(ArrayRef<StringRef> InputFiles, const ArgList &Args) {
       *TempFileOrErr,
       Args.MakeArgString("--target=" + Triple.getTriple()),
       Triple.isAMDGPU() ? Args.MakeArgString("-mcpu=" + Arch)
-                        : Args.MakeArgString("-march=" + Arch),
-      Args.MakeArgString("-" + OptLevel),
+                        : Args.MakeArgString("-march=" + Arch)
   };
 
   // Forward all of the `--offload-opt` and similar options to the device.
-  CmdArgs.push_back("-flto");
   for (auto &Arg : Args.filtered(OPT_offload_opt_eq_minus, OPT_mllvm))
     CmdArgs.append(
         {"-Xlinker",
@@ -547,29 +544,12 @@ Expected<StringRef> clang(ArrayRef<StringRef> InputFiles, const ArgList &Args) {
     CmdArgs.append({"-Xlinker", Args.MakeArgString(
                                     "-mllvm=" + StringRef(Arg->getValue()))});
 
-  if (Args.hasArg(OPT_debug))
-    CmdArgs.push_back("-g");
-
-  if (SaveTemps)
-    CmdArgs.push_back("-save-temps");
-
   if (SaveTemps && linkerSupportsLTO(Args))
     CmdArgs.push_back("-Wl,--save-temps");
 
   if (Args.hasArg(OPT_embed_bitcode))
     CmdArgs.push_back("-Wl,--lto-emit-llvm");
 
-  if (Verbose)
-    CmdArgs.push_back("-v");
-
-  if (!CudaBinaryPath.empty())
-    CmdArgs.push_back(Args.MakeArgString("--cuda-path=" + CudaBinaryPath));
-
-  for (StringRef Arg : Args.getAllArgValues(OPT_ptxas_arg))
-    llvm::copy(
-        SmallVector<StringRef>({"-Xcuda-ptxas", Args.MakeArgString(Arg)}),
-        std::back_inserter(CmdArgs));
-
   for (StringRef Arg : Args.getAllArgValues(OPT_linker_arg_EQ))
     CmdArgs.append({"-Xlinker", Args.MakeArgString(Arg)});
   for (StringRef Arg : Args.getAllArgValues(OPT_compiler_arg_EQ))
diff --git a/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td b/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
index 57d918db0a73ced..748a99ef57ccc02 100644
--- a/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
+++ b/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
@@ -19,9 +19,6 @@ def cuda_path_EQ : Joined<["--"], "cuda-path=">,
 def host_triple_EQ : Joined<["--"], "host-triple=">,
   Flags<[WrapperOnlyOption]>, MetaVarName<"<triple>">,
   HelpText<"Triple to use for the host compilation">;
-def opt_level : Joined<["--"], "opt-level=">,
-  Flags<[WrapperOnlyOption]>, MetaVarName<"<O0, O1, O2, or O3>">,
-  HelpText<"Optimization level for LTO">;
 def device_linker_args_EQ : Joined<["--"], "device-linker=">,
   Flags<[WrapperOnlyOption]>, MetaVarName<"<value> or <triple>=<value>">,
   HelpText<"Arguments to pass to the device linker invocation">;
@@ -35,17 +32,6 @@ def verbose : Flag<["--"], "wrapper-verbose">,
   Flags<[WrapperOnlyOption]>, HelpText<"Verbose output from tools">;
 def embed_bitcode : Flag<["--"], "embed-bitcode">,
   Flags<[WrapperOnlyOption]>, HelpText<"Embed linked bitcode in the module">;
-def debug : Flag<["--"], "device-debug">, Flags<[WrapperOnlyOption]>, 
-  HelpText<"Use debugging">;
-def ptxas_arg : Joined<["--"], "ptxas-arg=">,
-  Flags<[WrapperOnlyOption]>,
-  HelpText<"Argument to pass to the 'ptxas' invocation">;
-def pass_remarks_EQ : Joined<["--"], "pass-remarks=">,
-  Flags<[WrapperOnlyOption]>, HelpText<"Pass remarks for LTO">;
-def pass_remarks_missed_EQ : Joined<["--"], "pass-remarks-missed=">,
-  Flags<[WrapperOnlyOption]>, HelpText<"Pass remarks for LTO">;
-def pass_remarks_analysis_EQ : Joined<["--"], "pass-remarks-analysis=">,
-  Flags<[WrapperOnlyOption]>, HelpText<"Pass remarks for LTO">;
 def print_wrapped_module : Flag<["--"], "print-wrapped-module">,
   Flags<[WrapperOnlyOption]>,
   HelpText<"Print the wrapped module's IR for testing">;

Copy link

github-actions bot commented Feb 7, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Summary:
Recent changes made a lot of this stuff redundant or unused, clean it up
a bit.

Also snuck in a change to pass the CUDA path since we still use it for
`fatbinary` internally.
@jhuber6 jhuber6 merged commit addbb44 into llvm:main Feb 7, 2025
6 of 7 checks passed
@jhuber6 jhuber6 deleted the CleanupOptions branch February 7, 2025 21:40
@jplehr
Copy link
Contributor

jplehr commented Feb 10, 2025

This broke our flang-enabled buildbot https://lab.llvm.org/staging/#/builders/105/builds/15554
Looking at it currently

jplehr added a commit that referenced this pull request Feb 10, 2025
jplehr added a commit that referenced this pull request Feb 10, 2025
…26495)

Reverts #126297

Broken buildbots
https://lab.llvm.org/staging/#/builders/105/builds/15554
https://lab.llvm.org/buildbot/#/builders/30/builds/15490

Error is
```
# .---command stderr------------
# | FileCheck error: '/work/janplehr/git/llvm-project/bot-tester-builds/cmakecachebuild/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/bug51781.c.tmp.custom' is empty.
# | FileCheck command line:  /home/janplehr/git/llvm-project/bot-tester-builds/cmakecachebuild/./bin/FileCheck /work/janplehr/git/llvm-project/offload/test/offloading/bug51781.c -check-prefix=CUSTOM -input-file=/work/janplehr/git/llvm-project/bot-tester-builds/cmakecachebuild/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/bug51781.c.tmp.custom
```
The file is empty, while the `CUSTOM` check-target expects to find
```
// CUSTOM: Rewriting generic-mode kernel with a customized state machine.
```
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Feb 10, 2025
…arding" (#126495)

Reverts llvm/llvm-project#126297

Broken buildbots
https://lab.llvm.org/staging/#/builders/105/builds/15554
https://lab.llvm.org/buildbot/#/builders/30/builds/15490

Error is
```
# .---command stderr------------
# | FileCheck error: '/work/janplehr/git/llvm-project/bot-tester-builds/cmakecachebuild/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/bug51781.c.tmp.custom' is empty.
# | FileCheck command line:  /home/janplehr/git/llvm-project/bot-tester-builds/cmakecachebuild/./bin/FileCheck /work/janplehr/git/llvm-project/offload/test/offloading/bug51781.c -check-prefix=CUSTOM -input-file=/work/janplehr/git/llvm-project/bot-tester-builds/cmakecachebuild/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/bug51781.c.tmp.custom
```
The file is empty, while the `CUSTOM` check-target expects to find
```
// CUSTOM: Rewriting generic-mode kernel with a customized state machine.
```
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
Summary:
Recent changes made a lot of this stuff redundant or unused, clean it up
a bit.

Also snuck in a change to pass the CUDA path since we still use it for
`fatbinary` internally.
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…vm#126495)

Reverts llvm#126297

Broken buildbots
https://lab.llvm.org/staging/#/builders/105/builds/15554
https://lab.llvm.org/buildbot/#/builders/30/builds/15490

Error is
```
# .---command stderr------------
# | FileCheck error: '/work/janplehr/git/llvm-project/bot-tester-builds/cmakecachebuild/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/bug51781.c.tmp.custom' is empty.
# | FileCheck command line:  /home/janplehr/git/llvm-project/bot-tester-builds/cmakecachebuild/./bin/FileCheck /work/janplehr/git/llvm-project/offload/test/offloading/bug51781.c -check-prefix=CUSTOM -input-file=/work/janplehr/git/llvm-project/bot-tester-builds/cmakecachebuild/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/bug51781.c.tmp.custom
```
The file is empty, while the `CUSTOM` check-target expects to find
```
// CUSTOM: Rewriting generic-mode kernel with a customized state machine.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants