[clang] [Clang] Simplify specifying passes via -Xoffload-linker (PR #102483)
Joel E. Denny via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 8 08:19:34 PDT 2024
https://github.com/jdenny-ornl updated https://github.com/llvm/llvm-project/pull/102483
>From 9e8a8e78f3014324d8aa35dd1615b3f5720a5cb4 Mon Sep 17 00:00:00 2001
From: "Joel E. Denny" <jdenny.ornl at gmail.com>
Date: Wed, 7 Aug 2024 19:10:33 -0400
Subject: [PATCH 1/2] [Clang] Simplify specifying passes via -Xoffload-linker
Make it possible to do things like the following, regardless of
whether `$GPU_ARCH` is for nvptx or amdgpu:
```
$ clang -O1 -g -fopenmp -fopenmp-targets=$GPU_ARCH test.c \
-Xoffload-linker -mllvm=-pass-remarks=inline \
-Xoffload-linker -mllvm=-force-remove-attribute=g.internalized:noinline\
-Xoffload-linker --lto-newpm-passes='forceattrs,default<O1>' \
-Xoffload-linker --lto-debug-pass-manager \
-foffload-lto
```
To accomplish that:
- In clang-linker-wrapper, do not forward options via `-Wl` if they
might have literal commas. Use `-Xlinker` instead.
- In clang-nvlink-wrapper, accept `--lto-debug-pass-manager` and
`--lto-newpm-passes`.
- In clang-nvlink-wrapper, drop `-passes` because it's inconsistent
with the interface of `lld`, which is used instead of
clang-nvlink-wrapper when the target is amdgpu. Without this patch,
`-passes` is passed to `nvlink`, producing an error anyway.
---
clang/test/Driver/linker-wrapper.c | 18 ++++++++++++------
clang/test/Driver/nvlink-wrapper.c | 8 ++++++++
.../ClangLinkerWrapper.cpp | 12 ++++++++----
.../ClangNVLinkWrapper.cpp | 15 ++-------------
clang/tools/clang-nvlink-wrapper/NVLinkOpts.td | 8 ++++++++
5 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/clang/test/Driver/linker-wrapper.c b/clang/test/Driver/linker-wrapper.c
index 99cfdb9ebfc7cd..e70715d2a9bd7e 100644
--- a/clang/test/Driver/linker-wrapper.c
+++ b/clang/test/Driver/linker-wrapper.c
@@ -238,9 +238,15 @@ __attribute__((visibility("protected"), used)) int x;
// RUN: clang-offload-packager -o %t.out \
// RUN: --image=file=%t.elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908
// 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 --offload-opt=-pass-remarks=foo \
-// RUN: --linker-path=/usr/bin/ld %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=OFFLOAD-OPT
-// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run -mllvm -pass-remarks=foo \
-// RUN: --linker-path=/usr/bin/ld %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=OFFLOAD-OPT
-
-// OFFLOAD-OPT: clang{{.*}}-Wl,--plugin-opt=-pass-remarks=foo
+// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
+// RUN: --offload-opt=-pass-remarks=foo,bar --linker-path=/usr/bin/ld \
+// RUN: %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=OFFLOAD-OPT
+// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
+// RUN: -mllvm -pass-remarks=foo,bar --linker-path=/usr/bin/ld \
+// RUN: %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=MLLVM
+
+// MLLVM: clang{{.*}}-Xlinker --plugin-opt=-pass-remarks=foo,bar
+// OFFLOAD-OPT: clang{{.*}}-Xlinker --plugin-opt=-pass-remarks=foo,bar
+// MLLVM-SAME: -Xlinker -mllvm=-pass-remarks=foo,bar
+// OFFLOAD-OPT-NOT: -Xlinker -mllvm=-pass-remarks=foo,bar
+// OFFLOAD-OPT-SAME: {{$}}
diff --git a/clang/test/Driver/nvlink-wrapper.c b/clang/test/Driver/nvlink-wrapper.c
index 318315ddaca340..5d835d8d6cb2a2 100644
--- a/clang/test/Driver/nvlink-wrapper.c
+++ b/clang/test/Driver/nvlink-wrapper.c
@@ -70,3 +70,11 @@ int baz() { return y + x; }
// RUN: clang-nvlink-wrapper --dry-run %t.o %t-u.o %t-y.a \
// RUN: -arch sm_52 --cuda-path/opt/cuda -o a.out 2>&1 | FileCheck %s --check-prefix=PATH
// PATH-NOT: --cuda-path=/opt/cuda
+
+//
+// Check that passes can be specified and debugged.
+//
+// RUN: clang-nvlink-wrapper --dry-run %t.o %t-u.o %t-y.a \
+// RUN: --lto-debug-pass-manager --lto-newpm-passes=forceattrs \
+// RUN: -arch sm_52 -o a.out 2>&1 | FileCheck %s --check-prefix=PASSES
+// PASSES: Running pass: ForceFunctionAttrsPass
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 24cc4f0eeadf91..7030556ce01b19 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -527,9 +527,11 @@ Expected<StringRef> clang(ArrayRef<StringRef> InputFiles, const ArgList &Args) {
// Forward all of the `--offload-opt` and similar options to the device.
if (linkerSupportsLTO(Args)) {
- for (auto &Arg : Args.filtered(OPT_offload_opt_eq_minus, OPT_mllvm))
+ for (auto &Arg : Args.filtered(OPT_offload_opt_eq_minus, OPT_mllvm)) {
+ CmdArgs.push_back(Args.MakeArgString("-Xlinker"));
CmdArgs.push_back(
- Args.MakeArgString("-Wl,--plugin-opt=" + StringRef(Arg->getValue())));
+ Args.MakeArgString("--plugin-opt=" + StringRef(Arg->getValue())));
+ }
}
if (!Triple.isNVPTX())
@@ -571,9 +573,11 @@ Expected<StringRef> clang(ArrayRef<StringRef> InputFiles, const ArgList &Args) {
}
// Pass on -mllvm options to the linker invocation.
- for (const opt::Arg *Arg : Args.filtered(OPT_mllvm))
+ for (const opt::Arg *Arg : Args.filtered(OPT_mllvm)) {
+ CmdArgs.push_back(Args.MakeArgString("-Xlinker"));
CmdArgs.push_back(
- Args.MakeArgString("-Wl,-mllvm=" + StringRef(Arg->getValue())));
+ Args.MakeArgString("-mllvm=" + StringRef(Arg->getValue())));
+ }
if (Args.hasArg(OPT_debug))
CmdArgs.push_back("-g");
diff --git a/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp b/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
index 7851414ba7f4dd..871fe5e4553ccb 100644
--- a/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
+++ b/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
@@ -84,18 +84,6 @@ static cl::list<std::string>
PassPlugins("load-pass-plugin",
cl::desc("Load passes from plugin library"));
-static cl::opt<std::string> PassPipeline(
- "passes",
- cl::desc(
- "A textual description of the pass pipeline. To have analysis passes "
- "available before a certain pass, add 'require<foo-analysis>'. "
- "'-passes' overrides the pass pipeline (but not all effects) from "
- "specifying '--opt-level=O?' (O2 is the default) to "
- "clang-linker-wrapper. Be sure to include the corresponding "
- "'default<O?>' in '-passes'."));
-static cl::alias PassPipeline2("p", cl::aliasopt(PassPipeline),
- cl::desc("Alias for -passes"));
-
static void printVersion(raw_ostream &OS) {
OS << clang::getClangToolFullVersion("clang-nvlink-wrapper") << '\n';
}
@@ -365,8 +353,9 @@ Expected<std::unique_ptr<lto::LTO>> createLTO(const ArgList &Args) {
Conf.OptLevel = Args.getLastArgValue(OPT_O, "2")[0] - '0';
Conf.DefaultTriple = Triple.getTriple();
- Conf.OptPipeline = PassPipeline;
+ Conf.OptPipeline = Args.getLastArgValue(OPT_lto_newpm_passes, "");
Conf.PassPlugins = PassPlugins;
+ Conf.DebugPassManager = Args.hasArg(OPT_lto_debug_pass_manager);
Conf.DiagHandler = diagnosticHandler;
Conf.CGFileType = CodeGenFileType::AssemblyFile;
diff --git a/clang/tools/clang-nvlink-wrapper/NVLinkOpts.td b/clang/tools/clang-nvlink-wrapper/NVLinkOpts.td
index 01bd0f85b1a33e..a97190b7a614cc 100644
--- a/clang/tools/clang-nvlink-wrapper/NVLinkOpts.td
+++ b/clang/tools/clang-nvlink-wrapper/NVLinkOpts.td
@@ -1,3 +1,6 @@
+// We try to create options similar to lld's. That way, options passed to clang
+// -Xoffload-linker can be the same whether offloading to nvptx or amdgpu.
+
include "llvm/Option/OptParser.td"
def WrapperOnlyOption : OptionFlag;
@@ -70,6 +73,11 @@ def plugin_opt : Joined<["--", "-"], "plugin-opt=">, Flags<[WrapperOnlyOption]>,
HelpText<"Options passed to LLVM, not including the Clang invocation. Use "
"'--plugin-opt=--help' for a list of options.">;
+def lto_newpm_passes : Joined<["--"], "lto-newpm-passes=">,
+ Flags<[WrapperOnlyOption]>, HelpText<"Passes to run during LTO">;
+def lto_debug_pass_manager : Flag<["--"], "lto-debug-pass-manager">,
+ Flags<[WrapperOnlyOption]>, HelpText<"Debug new pass manager">;
+
def save_temps : Flag<["--", "-"], "save-temps">,
Flags<[WrapperOnlyOption]>, HelpText<"Save intermediate results">;
>From d7dc3e2c6b175b804ef129a5faad9f7d15e71240 Mon Sep 17 00:00:00 2001
From: "Joel E. Denny" <jdenny.ornl at gmail.com>
Date: Thu, 8 Aug 2024 11:17:18 -0400
Subject: [PATCH 2/2] Apply reviewer suggestion
---
.../clang-linker-wrapper/ClangLinkerWrapper.cpp | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 7030556ce01b19..142c55ddd2898b 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -527,11 +527,10 @@ Expected<StringRef> clang(ArrayRef<StringRef> InputFiles, const ArgList &Args) {
// Forward all of the `--offload-opt` and similar options to the device.
if (linkerSupportsLTO(Args)) {
- for (auto &Arg : Args.filtered(OPT_offload_opt_eq_minus, OPT_mllvm)) {
- CmdArgs.push_back(Args.MakeArgString("-Xlinker"));
- CmdArgs.push_back(
- Args.MakeArgString("--plugin-opt=" + StringRef(Arg->getValue())));
- }
+ for (auto &Arg : Args.filtered(OPT_offload_opt_eq_minus, OPT_mllvm))
+ CmdArgs.append(
+ {"-Xlinker",
+ Args.MakeArgString("--plugin-opt=" + StringRef(Arg->getValue()))});
}
if (!Triple.isNVPTX())
@@ -574,9 +573,8 @@ Expected<StringRef> clang(ArrayRef<StringRef> InputFiles, const ArgList &Args) {
// Pass on -mllvm options to the linker invocation.
for (const opt::Arg *Arg : Args.filtered(OPT_mllvm)) {
- CmdArgs.push_back(Args.MakeArgString("-Xlinker"));
- CmdArgs.push_back(
- Args.MakeArgString("-mllvm=" + StringRef(Arg->getValue())));
+ CmdArgs.append({"-Xlinker", Args.MakeArgString(
+ "-mllvm=" + StringRef(Arg->getValue()))});
}
if (Args.hasArg(OPT_debug))
More information about the cfe-commits
mailing list