[PATCH] D147506: [DebugLine] save one debug line entry for empty prologue

Mon Apr 24 14:31:13 PDT 2023

dblaikie added a comment.

In D147506#4291043 <https://reviews.llvm.org/D147506#4291043>, @shchenz wrote:

> Added a simple assert in `MCObjectStreamer::emitDwarfAdvanceLineAddr()` for the absolute address diffs, and run a unittest check in llvm. All the failures are for assembly file (hardcode two `.loc` without any "real" instructions between). The `ll` input can not produce 0 address diff.

In D147506#4291031 <https://reviews.llvm.org/D147506#4291031>, @shchenz wrote:

> Hi @dblaikie , in the MCStreamer when debug line is handled in `DwarfDebug::beginInstruction()`, it already ignores the case for zero length entries, see https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp#L2020-L2024 . `DwarfDebug::beginInstruction()` is called for each instruction and if the instruction is not a "real" instruction on the platform(which means there will be zero length entry in the debug line section), no debug line entry will be added. If the instruction is a "real" instruction, then the address must be changed. So, seems `emitInitialLocDirective()` is the only place where zero length entry may be generated. What do you think? Thank you!

I'd still prefer this to be at a lower layer - because I worry we'll end up missing cases (we have prologue and DwarfDebug so far - maybe we'll end up with a case in some other situation like epilogues, or some other "special case")
Like maybe `MCObjectStreamer::emitDwarfADvanceLineAddr()` could be a non-virtual wrapper up in `MCStreamer` that delegates to the virtual one (asm or object) and handles the zero-length skipping, probably under a flag (if we need to preserve the behavior/opt-in to the behavior when we're assembling - though I'm not 100% sure we need to bother with that degree of compatibility either - I think we'd be doing the DWARF community a service by removing them earlier and removing the confusion/debate they create later on - but that opinion might be too strong/opinionated)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147506