This is an archive of the discontinued LLVM Phabricator instance.

[Clang] Remove unused -split-dwarf and obsolete -enable-split-dwarf
ClosedPublic

Authored by aaronpuchert on Jun 11 2019, 2:09 PM.

Details

Summary

The changes in D59673 made the choice redundant, since we can achieve
single-file split DWARF just by not setting an output file name.
Like llc we can also derive whether to enable Split DWARF from whether
-split-dwarf-file is set, so we don't need the flag at all anymore.

The test CodeGen/split-debug-filename.c distinguished between having set
or not set -enable-split-dwarf with -split-dwarf-file, but we can
probably just always emit the metadata into the IR.

The flag -split-dwarf wasn't used at all anymore.

Diff Detail

Repository
rL LLVM

Event Timeline

aaronpuchert created this revision.Jun 11 2019, 2:09 PM
dblaikie added inline comments.Jun 11 2019, 5:57 PM
include/clang/Basic/CodeGenOptions.def
263 ↗(On Diff #204162)

Could we skip this and rely on "SplitDwarFile" being non-empty?

aaronpuchert marked an inline comment as done.Jun 12 2019, 5:30 AM
aaronpuchert added inline comments.
include/clang/Basic/CodeGenOptions.def
263 ↗(On Diff #204162)

I think not, have a look at test/CodeGen/split-debug-filename.c:

// RUN: %clang_cc1 -debug-info-kind=limited -split-dwarf-file foo.dwo -S -emit-llvm -o - %s | FileCheck %s
// RUN: %clang_cc1 -debug-info-kind=limited -enable-split-dwarf -split-dwarf-file foo.dwo -S -emit-llvm -o - %s | FileCheck --check-prefix=VANILLA %s
// ...

// Testing to ensure that the dwo name gets output into the compile unit.
// CHECK: !DICompileUnit({{.*}}, splitDebugFilename: "foo.dwo"

// Testing to ensure that the dwo name is not output into the compile unit if
// it's for vanilla split-dwarf rather than split-dwarf for implicit modules.
// VANILLA-NOT: splitDebugFilename

This seems intentional.

aaronpuchert added inline comments.Jun 13 2019, 12:41 PM
include/clang/Basic/CodeGenOptions.def
263 ↗(On Diff #204162)

Although if that's up to change, I think we can drop it. If I'm not overlooking anything, -enable-split-dwarf does nothing when -emit-llvm is also given. So I'm not sure we lose anything by treating it the same way as -emit-llvm -split-dwarf-file ... without -enable-split-dwarf.

Actually llc has exactly that behavior (inferring whether to enable Split DWARF depending on whether there was a -split-dwarf-file argument), as I remarked in D59673#1457720, but it doesn't emit IR and doesn't have to deal with implicit modules...

Remove -enable-split-dwarf completely, adapting the test case accordingly. Also remove -split-dwarf which wasn't used at all.

aaronpuchert marked 2 inline comments as done.Jun 15 2019, 9:17 AM
aaronpuchert retitled this revision from [Clang] Remove obsolete -enable-split-dwarf={single,split} to [Clang] Remove unused -split-dwarf and obsolete -enable-split-dwarf.Jun 15 2019, 9:19 AM
aaronpuchert edited the summary of this revision. (Show Details)
aaronpuchert added inline comments.Jun 15 2019, 9:22 AM
clang/lib/CodeGen/BackendUtil.cpp
864 ↗(On Diff #204928)

Perhaps I should also check whether SplitDwarfFile is empty.

1276 ↗(On Diff #204928)

Dito here.

clang/test/CodeGen/split-debug-filename.c
14–16 ↗(On Diff #204928)

We lose that when we remove -enable-split-dwarf.

aaronpuchert added inline comments.Jun 25 2019, 5:10 PM
clang/lib/CodeGen/BackendUtil.cpp
864 ↗(On Diff #204928)

Maybe not. As it is, we just create an empty (but valid) debug info file when we have -split-dwarf-output but no -split-dwarf-file. This is exactly what llc does as well.

$ clang -c -g -o clang.o -Xclang -split-dwarf-output -Xclang clang.dwo test.cpp
$ clang -c -S -emit-llvm -g -o test.ll test.cpp
$ llc -filetype=obj -o llc.o -split-dwarf-output llc.dwo test.ll
$ diff <(llvm-dwarfdump clang.o) <(llvm-dwarfdump llc.o)
1c1
< clang.o:      file format ELF64-x86-64
---
> llc.o:        file format ELF64-x86-64
$ diff clang.dwo llc.dwo
dblaikie accepted this revision.Jun 26 2019, 8:44 AM

Sounds alright :)

This revision is now accepted and ready to land.Jun 26 2019, 8:44 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 26 2019, 2:36 PM

Thanks for the reviews!