[PATCH] D76293: [WIP][DebugInfo] refactor DIE classes. Remove dependency on AsmPrinter.

Tue Mar 17 20:00:02 PDT 2020

dblaikie added a comment.

> During work on "Remove obsolete debug info in lld" I need to generate DWARF without using AsmPrinter(To not to add extra libraries to lld).

Could you point to where this is a requirement? (has this already been discussed in review/found to be an unacceptable dependency? llvm-dsymutil uses the existing AsmPrinter-based APIs to write the output, yes?)

> I think it would be a bad idea to write a new set of DWARF generating classes, not depending on AsmPrinter.

Agreed

>   Instead, it would be better to reduce dependence on AsmPrinter for current structures.

Agreed, if the dependency is unacceptable.

> There is already a place which implements such separation :
> 
> AsmPrinter/ByteStreamer.h:ByteStreamer
>  AsmPrinter/DwarfDebug.h:DwarfDebug::emitDebugLocEntry(ByteStreamer &Streamer)

For historics: That API was added by @echristo to allow both emitting the bytes and hashing the bytes of a location - so it does represent a small instance of the abstraction you're looking for, yes.

> But it is used locally. I think the same idea could be implemented in a more general way:
> 
> There should be a class that allows writing byte data in the dwarf section: DwarfDebugSection. DwarfDebugSection would have two descendands: DwarfDebugSectionAsm, DwarfDebugSectionBin.

I'd expect it, perhaps, to be more like ByteStreamer - which has a hashing and an AsmPrinter implementation. Though in this case we'd start with only an AsmPrinter implementation (AsmPrinter already abstracts over the asm/binary distinction, so there's hopefully no need to float that up/duplicate that in the DwarfDebugSection abstraction).

(I'm leaving naming mostly aside here for now - that'll have to be iterated on at some point)

> There should be a class that allows writing DWARF constructions(units, line tables, address ranges) to DwarfDebugSection: DwarfDebugFile.

Not quite sure what you're picturing with DwarfDebugFile - I guess you mean an abstraction over a collection of DwarfDebugSections related to a particular file (eg: one for a .o file, one for a .dwo file, perhaps).

Though looking at the API for DwarfDebugFile in your proposed patch, it seems to have features I'd consider too high level for that abstraction - like writeCompileUnitHeader etc - anything that is higher level than assembly (so ulebs, ints, relocations, labels) seems inappropriate for the abstraction. AsmPrinter should be the first/trivially wrapped in this abstraction, I think. (I wouldn't mind something less stateful, though - as you say, the section V file abstraction, though that might lead to some confusing assembly - since AsmPrinter is a streaming abstraction, so if you have a stateful section then the usage code might end up thinking it's totally fine to write a few things to one section, then a few things to another section - which creates for less-than-ideal assembly (fragmented sections) & so I think that'd be unfortunate)

> DIE* classes from CodeGen/DIE.h should be moved into DebugInfoDWARF library.

I'd say it'd be best to not move it into the DebugInfoDWARF library - that'd have the reverse effect, then LLVM's code generation would end up depending on DWARF parsing code it has no need for... - so I guess this'd need to go in a separate library.

The general idea of making these APIs independent of AsmPrinter - yeah, seems plausible, if it's especially necessary/useful - see the first questions.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76293/new/

https://reviews.llvm.org/D76293