From dbedcfdc2007e235d97b38db7693bd1f5ff443d8 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Tue, 9 May 2023 14:43:46 -0700 Subject: [PATCH] [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking When the final phase is linking, Clang currently places `.dwo` files in the current directory (like the `-c` behavior for multiple inputs). Strangely, -fdebug-compilation-dir=/-ffile-compilation-dir= is considered, which is untested. GCC has a more useful behavior that derives auxiliary filenames from the final output (-o). ``` gcc -c -g -gsplit-dwarf d/a.c d/b.c # a.dwo b.dwo gcc -g -gsplit-dwarf d/a.c d/b.c -o e/x # e/x-a.dwo e/x-b.dwo gcc -g -gsplit-dwarf d/a.c d/b.c # a-a.dwo a-b.dwo ``` Port a useful subset of GCC behaviors that are easy to describe to Clang. * Add a driver and cc1 option -dumpdir * When the final phase is link, add a default -dumpdir if not specified by the user * Forward -dumpdir to -cc1 command lines * tools::SplitDebugName prefers -dumpdir when constructing the .dwo filename GCC provides -dumpbase. If we use just one of -dumpdir and -dumpbase, -dumpbase isn't very useful as it appends a dash. ``` gcc -g -gsplit-dwarf -dumpdir e d/a.c # ea.dwo gcc -g -gsplit-dwarf -dumpdir e/ d/a.c # e/a.dwo gcc -g -gsplit-dwarf -dumpbase e d/a.c # e-a.dwo gcc -g -gsplit-dwarf -dumpbase e/ d/a.c # e/-a.dwo ``` If we specify both `-dumpdir` and `-dumpbase`, we can avoid the influence of the source filename when there is one input file. ``` gcc -g -gsplit-dwarf -dumpdir f/ -dumpbase x d/a.c # f/x.dwo gcc -g -gsplit-dwarf -dumpdir f/ -dumpbase x d/a.c d/b.c # f/x-a.dwo f/x-b.dwo ``` Given the above examples, I think -dumpbase is not useful. GCC -save-temps has interesting interaction with -dumpdir as -save-temps generated files are considered auxiliary files like .dwo files. For Clang, with this patch, -save-temps and -dumpdir are orthogonal, which is easier to explain. ``` gcc -g -gsplit-dwarf d/a.c -o e/x -dumpdir f/ -save-temps=obj # e/a.{i,s,o,dwo} gcc -g -gsplit-dwarf d/a.c -o e/x -save-temps=obj -dumpdir f/ # f/a.{i,s,o,dwo} clang -g -gsplit-dwarf d/a.c -o e/x -save-temps=obj -dumpdir f/ # e/a.{i,s,o} f/a.dwo ``` Reviewed By: dblaikie Differential Revision: https://reviews.llvm.org/D149193 --- clang/docs/ReleaseNotes.rst | 8 +++++ clang/include/clang/Driver/Options.td | 4 +++ clang/lib/Driver/Driver.cpp | 15 +++++++++ clang/lib/Driver/ToolChains/Clang.cpp | 2 ++ clang/lib/Driver/ToolChains/CommonArgs.cpp | 31 ++++++++++--------- .../test/Driver/hip-gsplit-dwarf-options.hip | 8 +++-- clang/test/Driver/split-debug.c | 26 ++++++++++++++-- 7 files changed, 75 insertions(+), 19 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 4ec4e4a49de6..c0820bd01fd5 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -192,12 +192,20 @@ New Compiler Flags - The flag ``-std=c++23`` has been added. This behaves the same as the existing flag ``-std=c++2b``. +- ``-dumpdir`` has been implemented to specify auxiliary and dump output + filenames for features like ``-gsplit-dwarf``. + Deprecated Compiler Flags ------------------------- Modified Compiler Flags ----------------------- +- ``clang -g -gsplit-dwarf a.c -o obj/x`` (compile and link) now generates the + ``.dwo`` file at ``obj/x-a.dwo``, instead of a file in the temporary + directory (``/tmp`` on \*NIX systems, if none of the environment variables + TMPDIR, TMP, and TEMP are specified). + Removed Compiler Flags ------------------------- - The deprecated flag `-fmodules-ts` is removed. Please use ``-std=c++20`` diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 496264b74d46..670003a3da84 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -1142,6 +1142,10 @@ def module_dependency_dir : Separate<["-"], "module-dependency-dir">, def dsym_dir : JoinedOrSeparate<["-"], "dsym-dir">, Flags<[NoXarchOption, RenderAsInput]>, HelpText<"Directory to output dSYM's (if any) to">, MetaVarName<"">; +// GCC style -dumpdir. We intentionally don't implement the less useful -dumpbase{,-ext}. +def dumpdir : Separate<["-"], "dumpdir">, Flags<[CC1Option]>, + MetaVarName<"">, + HelpText<"Use as a prefix to form auxiliary and dump file names">; def dumpmachine : Flag<["-"], "dumpmachine">; def dumpspecs : Flag<["-"], "dumpspecs">, Flags<[Unsupported]>; def dumpversion : Flag<["-"], "dumpversion">; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index a2deb9b0dd9a..879f3bf00d1c 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -3878,6 +3878,21 @@ void Driver::handleArguments(Compilation &C, DerivedArgList &Args, !Args.getLastArgValue(options::OPT_fuse_ld_EQ) .equals_insensitive("lld")) Diag(clang::diag::err_drv_lto_without_lld); + + // If -dumpdir is not specified, give a default prefix derived from the link + // output filename. For example, `clang -g -gsplit-dwarf a.c -o x` passes + // `-dumpdir x-` to cc1. If -o is unspecified, use + // stem(getDefaultImageName()) (usually stem("a.out") = "a"). + if (!Args.hasArg(options::OPT_dumpdir)) { + Arg *Arg = Args.MakeSeparateArg( + nullptr, getOpts().getOption(options::OPT_dumpdir), + Args.MakeArgString(Args.getLastArgValue( + options::OPT_o, + llvm::sys::path::stem(getDefaultImageName())) + + "-")); + Arg->claim(); + Args.append(Arg); + } } if (FinalPhase == phases::Preprocess || Args.hasArg(options::OPT__SLASH_Y_)) { diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index c12a6ab88097..a518cc487cfd 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -4854,6 +4854,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, } } + Args.AddLastArg(CmdArgs, options::OPT_dumpdir); + if (const Arg *A = Args.getLastArg(options::OPT_fthinlto_index_EQ)) { if (!types::isLLVMIR(Input.getType())) D.Diag(diag::err_drv_arg_requires_bitcode_input) << A->getAsString(Args); diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index cf21d6243b18..a6247b4cbe4e 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1248,23 +1248,24 @@ const char *tools::SplitDebugName(const JobAction &JA, const ArgList &Args, if (StringRef(A->getValue()) == "single") return Args.MakeArgString(Output.getFilename()); - Arg *FinalOutput = Args.getLastArg(options::OPT_o); - if (FinalOutput && Args.hasArg(options::OPT_c)) { - SmallString<128> T(FinalOutput->getValue()); - llvm::sys::path::remove_filename(T); - llvm::sys::path::append(T, llvm::sys::path::stem(FinalOutput->getValue())); - AddPostfix(T); - return Args.MakeArgString(T); + SmallString<128> T; + if (const Arg *A = Args.getLastArg(options::OPT_dumpdir)) { + T = A->getValue(); } else { - // Use the compilation dir. - Arg *A = Args.getLastArg(options::OPT_ffile_compilation_dir_EQ, - options::OPT_fdebug_compilation_dir_EQ); - SmallString<128> T(A ? A->getValue() : ""); - SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput())); - AddPostfix(F); - T += F; - return Args.MakeArgString(T); + Arg *FinalOutput = Args.getLastArg(options::OPT_o); + if (FinalOutput && Args.hasArg(options::OPT_c)) { + T = FinalOutput->getValue(); + llvm::sys::path::remove_filename(T); + llvm::sys::path::append(T, + llvm::sys::path::stem(FinalOutput->getValue())); + AddPostfix(T); + return Args.MakeArgString(T); + } } + + T += llvm::sys::path::stem(Input.getBaseInput()); + AddPostfix(T); + return Args.MakeArgString(T); } void tools::SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T, diff --git a/clang/test/Driver/hip-gsplit-dwarf-options.hip b/clang/test/Driver/hip-gsplit-dwarf-options.hip index e9f87872da86..cfd5c5020e66 100644 --- a/clang/test/Driver/hip-gsplit-dwarf-options.hip +++ b/clang/test/Driver/hip-gsplit-dwarf-options.hip @@ -13,13 +13,17 @@ // RUN: %clang -### --target=x86_64-unknown-linux-gnu \ // RUN: --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \ // RUN: --offload-arch=gfx900 \ -// RUN: -ggdb -gsplit-dwarf 2>&1 | FileCheck %s +// RUN: -ggdb -gsplit-dwarf 2>&1 | FileCheck %s --check-prefix=LINK // RUN: %clang -### --target=x86_64-unknown-linux-gnu \ // RUN: -fgpu-rdc --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \ // RUN: --offload-arch=gfx900 \ -// RUN: -ggdb -gsplit-dwarf 2>&1 | FileCheck %s +// RUN: -ggdb -gsplit-dwarf 2>&1 | FileCheck %s --check-prefix=LINK // CHECK-DAG: {{".*clang.*".* "-target-cpu" "gfx906".* "-split-dwarf-output" "hip-gsplit-dwarf-options_gfx906:xnack\+.dwo"}} // CHECK-DAG: {{".*clang.*".* "-target-cpu" "gfx900".* "-split-dwarf-output" "hip-gsplit-dwarf-options_gfx900.dwo"}} // CHECK-DAG: {{".*clang.*".* "-target-cpu" "x86-64".* "-split-dwarf-output" "hip-gsplit-dwarf-options.dwo"}} + +// LINK-DAG: {{".*clang.*".* "-target-cpu" "gfx906".* "-split-dwarf-output" "a-hip-gsplit-dwarf-options_gfx906:xnack\+.dwo"}} +// LINK-DAG: {{".*clang.*".* "-target-cpu" "gfx900".* "-split-dwarf-output" "a-hip-gsplit-dwarf-options_gfx900.dwo"}} +// LINK-DAG: {{".*clang.*".* "-target-cpu" "x86-64".* "-split-dwarf-output" "a-hip-gsplit-dwarf-options.dwo"}} diff --git a/clang/test/Driver/split-debug.c b/clang/test/Driver/split-debug.c index 94db20b55404..19214f997c66 100644 --- a/clang/test/Driver/split-debug.c +++ b/clang/test/Driver/split-debug.c @@ -9,6 +9,7 @@ // INLINE: "-fsplit-dwarf-inlining" // NOINLINE-NOT: "-fsplit-dwarf-inlining" +// SPLIT-NOT: "-dumpdir" // SPLIT: "-debug-info-kind=constructor" // SPLIT-SAME: "-ggnu-pubnames" // SPLIT-SAME: "-split-dwarf-file" "split-debug.dwo" "-split-dwarf-output" "split-debug.dwo" @@ -54,8 +55,29 @@ // SINGLE_WITH_FILENAME: "-split-dwarf-file" "{{.*}}foo.o" // SINGLE_WITH_FILENAME-NOT: "-split-dwarf-output" -/// Without -c, clang performs linking as well. The output is unchanged. -// RUN: %clang -### -target x86_64-unknown-linux-gnu -gsplit-dwarf -g %s -o ignore.d 2>&1 | FileCheck %s --check-prefix=SPLIT +/// If linking is the final phase, the .dwo filename is derived from -o (if specified) or "a". +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -gsplit-dwarf -g %s -o obj/out 2>&1 | FileCheck %s --check-prefix=SPLIT_LINK +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -gsplit-dwarf -g %s 2>&1 | FileCheck %s --check-prefix=SPLIT_LINK_A + +// SPLIT_LINK: "-dumpdir" "obj/out-" +// SPLIT_LINK: "-debug-info-kind=constructor" +// SPLIT_LINK-SAME: "-split-dwarf-file" "obj/out-split-debug.dwo" "-split-dwarf-output" "obj/out-split-debug.dwo" +// SPLIT_LINK_A: "-dumpdir" "a-" +// SPLIT_LINK_A-SAME: "-split-dwarf-file" "a-split-debug.dwo" "-split-dwarf-output" "a-split-debug.dwo" + +/// GCC special cases /dev/null (HOST_BIT_BUCKET) but not other special files like /dev/zero. +/// We don't apply special rules at all. +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -gsplit-dwarf -g %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=SPLIT_LINK_NULL + +// SPLIT_LINK_NULL: "-dumpdir" "/dev/null-" +// SPLIT_LINK_NULL-SAME: "-split-dwarf-output" "/dev/null-split-debug.dwo" + +/// If -dumpdir is specified, use its value to derive the .dwo filename. +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -gsplit-dwarf -g %s -o obj/out -dumpdir pf/x -c 2>&1 | FileCheck %s --check-prefix=DUMPDIR +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -gsplit-dwarf -g %s -o obj/out -dumpdir pf/x 2>&1 | FileCheck %s --check-prefix=DUMPDIR + +// DUMPDIR: "-dumpdir" "pf/x" +// DUMPDIR-SAME: "-split-dwarf-output" "pf/xsplit-debug.dwo" /// -fsplit-dwarf-inlining // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=split -g -fsplit-dwarf-inlining %s 2>&1 | FileCheck %s --check-prefixes=INLINE,SPLIT -- 2.34.1