[PATCH] D54576: [clang] - Simplify tools::SplitDebugName.
George Rimar via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 15 05:14:10 PST 2018
grimar created this revision.
grimar added a reviewer: dblaikie.
grimar added a comment.
`SplitDebugName` is called from the following 4 places:
1. void Clang::ConstructJob()
(https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L3933)
We can get here when clang construct jobs for regular case (cpp files):
EX: main.cpp -g -gsplit-dwarf -o ooo
Clang by itself does not recognize the '-fdebug-compilation-dir':
clang main.cpp -g -gsplit-dwarf -o ooo -### -fdebug-compilation-dir
clang-8: error: unknown argument: '-fdebug-compilation-dir'
So I believe this option can not be used, be in Args
and hence affect on anything during this flow.
In other places clang constructs assembly jobs:
2. void ClangAs::ConstructJob()
(https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L5900)
Here it tries to use the OPT_fdebug_compilation_dir option from Args:
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/CommonArgs.cpp#L824
And this is used to construct the argument for -split-dwarf-file.
The following also would not work:
clang main.s -g -gsplit-dwarf -o ooo -fdebug-compilation-dir,xxx
clang-8: error: unknown argument: '-fdebug-compilation-dir,xxx'
So I do not see the way to add the '-fdebug-compilation-dir' to Args here too,
so `SplitDebugName` does not use the '-fdebug-compilation-dir' I think
and its logic is dead for this case too I believe)
3, 4) These are similar:
tools::gnutools::Assembler::ConstructJob()
(https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Gnu.cpp#L820)
tools::MinGW::Assembler::ConstructJob()
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/MinGW.cpp#L57
Both call `SplitDebugName` to construct calls for objcopy:
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/CommonArgs.cpp#L833
But again, `Args` for their `ConstructJob` seems can never contain the `-fdebug-compilation-dir`
I just do not see any way to achieve that.
Clang's code adds the `-fdebug-compilation-dir` by itself in `Clang::ConstructJob()` and
`ClangAs::ConstructJob()`, but it will never be used in `SplitDebugName` I think:
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L601
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L5795
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L4172
That `-fdebug-compilation-dir` is used in another places, not in the `SplitDebugName`
(used in invocations for -cc1 and -cc1as:
https://github.com/llvm-mirror/clang/blob/master/tools/driver/cc1as_main.cpp#L234
https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/CompilerInvocation.cpp#L944)
After changes done by this patch, no check-llvm/check-clang tests started to fail for me.
This should be NFC change.
`SplitDebugName` started to accept the `Output` that
can be used to simplify the logic a bit, also it
seems that code in `SplitDebugName` that uses
`OPT_fdebug_compilation_dir` is simply dead.
My understanding may be wrong because I am
not very familiar with clang, but I investigated this and
below in the comment, there are my
arguments for that.
https://reviews.llvm.org/D54576
Files:
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/CommonArgs.cpp
lib/Driver/ToolChains/CommonArgs.h
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/MinGW.cpp
Index: lib/Driver/ToolChains/MinGW.cpp
===================================================================
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -54,7 +54,7 @@
if (Args.hasArg(options::OPT_gsplit_dwarf))
SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
- SplitDebugName(Args, Inputs[0], Output));
+ SplitDebugName(Args, Output));
}
void tools::MinGW::Linker::AddLibGCC(const ArgList &Args,
Index: lib/Driver/ToolChains/Gnu.cpp
===================================================================
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -817,7 +817,7 @@
if (Args.hasArg(options::OPT_gsplit_dwarf) &&
getToolChain().getTriple().isOSLinux())
SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
- SplitDebugName(Args, Inputs[0], Output));
+ SplitDebugName(Args, Output));
}
namespace {
Index: lib/Driver/ToolChains/CommonArgs.h
===================================================================
--- lib/Driver/ToolChains/CommonArgs.h
+++ lib/Driver/ToolChains/CommonArgs.h
@@ -63,7 +63,7 @@
const Tool &T);
const char *SplitDebugName(const llvm::opt::ArgList &Args,
- const InputInfo &Input, const InputInfo &Output);
+ const InputInfo &Output);
void SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
const JobAction &JA, const llvm::opt::ArgList &Args,
Index: lib/Driver/ToolChains/CommonArgs.cpp
===================================================================
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -808,26 +808,15 @@
return false;
}
-const char *tools::SplitDebugName(const ArgList &Args, const InputInfo &Input,
+const char *tools::SplitDebugName(const ArgList &Args,
const InputInfo &Output) {
if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
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::replace_extension(T, "dwo");
- return Args.MakeArgString(T);
- } else {
- // Use the compilation dir.
- SmallString<128> T(
- Args.getLastArgValue(options::OPT_fdebug_compilation_dir));
- SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
- llvm::sys::path::replace_extension(F, "dwo");
- T += F;
- return Args.MakeArgString(F);
- }
+ SmallString<128> T(Output.getFilename());
+ llvm::sys::path::replace_extension(T, "dwo");
+ return Args.MakeArgString(T);
}
void tools::SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
Index: lib/Driver/ToolChains/Clang.cpp
===================================================================
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3936,7 +3936,7 @@
const char *SplitDWARFOut;
if (SplitDWARF) {
CmdArgs.push_back("-split-dwarf-file");
- SplitDWARFOut = SplitDebugName(Args, Input, Output);
+ SplitDWARFOut = SplitDebugName(Args, Output);
CmdArgs.push_back(SplitDWARFOut);
}
@@ -5902,7 +5902,7 @@
if ((getDebugFissionKind(D, Args, A) == DwarfFissionKind::Split) &&
(T.isOSLinux() || T.isOSFuchsia())) {
CmdArgs.push_back("-split-dwarf-file");
- CmdArgs.push_back(SplitDebugName(Args, Input, Output));
+ CmdArgs.push_back(SplitDebugName(Args, Output));
}
assert(Input.isFilename() && "Invalid input.");
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54576.174193.patch
Type: text/x-patch
Size: 3742 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181115/a5e8793e/attachment.bin>
More information about the cfe-commits
mailing list