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.");
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.");
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits