This is an archive of the discontinued LLVM Phabricator instance.

[clang] - Simplify tools::SplitDebugName.
ClosedPublic

Authored by grimar on Nov 15 2018, 5:13 AM.

Details

Summary

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.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.EditedNov 15 2018, 5:13 AM

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 -debug-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 2 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 other 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.

dblaikie accepted this revision.Nov 15 2018, 10:56 AM

Agreed - looks like this went in untested in r175813 & has never been used.

This revision is now accepted and ready to land.Nov 15 2018, 10:56 AM
This revision was automatically updated to reflect the committed changes.