[PATCH] D49426: [DEBUG_INFO] fix .loc directives emitted for missing prologues

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

I'm OK with this change from a "slightly reduces debug info size" but I hesitate to suggest that this isn't a thing your consumer should support & this "When an assembly file with multiple .loc directives with no interveaning instructions are assembled, GAS produces a DWARF line table that maps one instruction to multiple entries." sounds like a bit of a mischaracterization of how I'd read the line table entries in question - I think this line table doesn't "map one instruction to multiple entries" but instead has an empty entry that contains no instructions. Everything between one .loc and the next is at that first location - if there's nothing between them, then nothing is at that location. I suspect that's how other consumers are viewing this situation.

If this is an invariant you want (that there are no zero-length ranges in the line table), maybe it makes sense to implement it as an option in the integrated assembler, rather than special casing this particular instance of zero-length ranges in the line table?

They seem useless enough, but important to reproduce accurately if that's what the assembly says and other assemblers portray that information - but for our own uses we can say "these are meaningless, elide any zero-length location range"?



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1343-1346
+      if (isPrologueInstruction(MI))
+        hasPrologue = true;
+      else
+        return std::make_pair(hasPrologue, MI.getDebugLoc());
----------------
I'd probably swap this around for an early return:

  if (!isPrologueInstruction(MI))
    return std::make_pair(...);
  hasPrologue = true;


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

https://reviews.llvm.org/D49426