[PATCH] D81145: [MC] Generate a compilation unit in the 64-bit DWARF format [3/7]

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/MC/MCDwarf.cpp:983
+  // The 4 (8 for DWARF64) byte offset to the debug abbrevs from the start of
+  // the .debug_abbrev, it is at the start of that section so this is zero.
   if (AbbrevSectionSymbol == nullptr)
----------------
Probably should be a full stop instead of a comma on this line, though I'm not 100% sure I entirely follow the comment as a whole.


================
Comment at: llvm/lib/MC/MCDwarf.cpp:985-987
+    MCOS->emitIntValue(0, OffsetSize);
   else
+    MCOS->emitSymbolValue(AbbrevSectionSymbol, OffsetSize,
----------------
There are two modified cases here, but only one test update for .debug_abbrev as far as I can see.  Does that indicate some missing coverage? Similar comment re. DW_AT_stmt_list below.


================
Comment at: llvm/lib/MC/MCDwarf.cpp:997-998
 
-  // DW_AT_stmt_list, a 4 byte offset from the start of the .debug_line section,
-  // which is at the start of that section so this is zero.
+  // DW_AT_stmt_list, a 4 (8 for DWARF64) byte offset from the start of the
+  // .debug_line section, which is at the start of that section so this is zero.
   if (LineSectionSymbol)
----------------
Again, this comment probably needs a bit of improvement.


================
Comment at: llvm/test/MC/ELF/gen-dwarf64.s:36-38
+    .section .foo, "ax", @progbits
+foo:
+    nop
----------------
Why has this been added?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81145