[llvm-dev] [Debuginfo][DWARF][LLD] Remove obsolete debug info in lld.

[David Blaikie via llvm-dev]

On Fri, Aug 14, 2020 at 9:50 AM Alexey Lapshin <avl.lapshin at gmail.com> wrote:
>
>
> On 10.08.2020 20:34, David Blaikie wrote:
> > On Mon, Aug 10, 2020 at 5:15 AM Alexey Lapshin <avl.lapshin at gmail.com> wrote:
> >> Hi Jonas,
> >>
> >> Thank you for the comments, please find my answers below...
> >>
> >> On 06.08.2020 20:39, Jonas Devlieghere wrote:
> >>
> >> Hi Alexey,
> >>
> >> I should've looked at this earlier. I went through the thread again and I've
> >> made some comments, mostly from the dsymutil point of view.
> >>
> >>> Current DWARFEmitter/DWARFStreamer has an implementation for DWARF
> >>> generation, which does not support DWARF5(only debug_names table). At the
> >>> same time, there already exists code in CodeGen/AsmPrinter/DwarfDebug.h,
> >>> which implements most of DWARF5. It seems that DWARFEmitter/DWARFStreamer
> >>> should be rewritten using DwarfDebug/DwarfFile. Though I am not sure
> >>> whether it would be easy to re-use DwarfDebug/DwarfFile. It would probably
> >>> be necessary to separate some intermediate level of DwarfDebug/DwarfFile.
> >> These classes serve very different purposes. Last time I looked at them there
> >> was very little overlap in functionality. In the compiler we're mostly
> >> concerned with generating the DWARF, while in dsymutil we try to copy
> >> everything we don't need to parse, and fix up what we have to. I don't want
> >> to say it's not possible, but I think supporting DWARF5 in those classes is
> >> going to be a lot less work than trying to reuse the CodeGen variants.
> >>
> >> I agree, in it`s current state it would be less work to write separate implementation
> >> than reusing CodeGen variants. The bad thing is that in such a case there is a lot of
> >> code duplication:
> >>
> >> DwarfStreamer::emitUnitRangesEntries
> >> DwarfDebug::emitDebugARanges
> >> EmitGenDwarfAranges
> >> DWARFYAML::emitDebugAranges
> > Probably some opportunities to share some code, even if not the whole
> > generator - might be best to refactor those opportunistically, rather
> > than a wholesale "change DWARFLinker to use (all) of
> > lib/CodeGen/AsmPrinter/Dwarf*". Sort of like the approach that's been
> > taken with lldb's use of libDebugInfoDWARF - picking particular
> > features that have high overlap and refactoring them to be reusable
> > between the two different use cases.
> One of the problem which complicates such a refactoring is that DWARF
> generation classes are closely coupled with AsmPrinter:
>
> llvm/CodeGen/DIE.h
>
> class DIE* {
>    void emitValue(const AsmPrinter *Asm, dwarf::Form Form) const;
>    unsigned SizeOf(const AsmPrinter *AP, dwarf::Form Form) const;
> }
>
> Having access to all of AsmPrinter public data members
> complicates DWARF generation:
>
> void DIEInlineString::emitValue(const AsmPrinter *AP, dwarf::Form Form)
> const {
>    if (Form == dwarf::DW_FORM_string) {
>      AP->OutStreamer->emitBytes(S);
>      AP->emitInt8(0);
>      return;
>    }
> }
>
> It would be good to do something similar to https://reviews.llvm.org/D76293.
> I.e. avoid AsmPrinter dependence using an abstract interface
> (llvm/DebugInfo/DWARF/DWARFDebugSection.h):
>
> class DIE* {
>    void emitValue(DwarfDebugSection *Dwarf, dwarf::Form Form) const;
>    unsigned SizeOf(const DwarfDebugSection *Dwarf, dwarf::Form Form) const;
> }
>
> Such separation, could f.e. allow to implement
> AsmPrinter::emitDwarfDIE(const DIE &Die)
> in some general place(libDebugInfoDWARF)

As mentioned in https://reviews.llvm.org/D76293#1928139 - probably
best not to put this into libDebugInfoDWARF - that library is
currently for DWARF parsing & LLVM proper only really needs DWARF
emission, so bundling them together may confuse things a bit - in
terms of adding unnecessary dependencies, conflating/confusing the
goals/priorities of the different libraries, etc. (see similar
separations between reading and writing with things like libIR
containing IR asm writing, but libAsmParser containing the parsing
code).

> and then be reused by others
> (without necessity to link/use AsmPrinter).
>
> https://reviews.llvm.org/D76293 was though to be more general.
> But we could probably start from that smaller change:
> avoid dependence of DIE* classes on AsmPrinter.

Sure - that'd be the general idea as we discussed in D76293 & in this
thread: start small, share some pieces & build up the necessary
abstractions (streaming APIs, etc) over the two different use cases,
etc.

>
> Alexey.
>
>
> >
> >> Supporting new standard would require rewriting/modification of all these places. In the ideal world,
> >> having single implementation for the DWARF generation allows changing one place and having
> >> benefits in others. Probably, CodeGen classes could be rewritten and then it would be useful
> >> to write them assuming two use cases - generation from the scratch and copying/updating
> >> existing data. In the end, there would be single implementation which could be reused in
> >> many places. Though, it is indeed a lot of work.
> >>
> >>
> >>