[PATCH] D80382: [MC] Implement generating 64-bit debug info.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Thanks for this. Looks pretty good, but I haven't given it enough of an in-depth read, nor is my MC knowledge in this area up-to-date enough to be confident enough to approve.



================
Comment at: llvm/tools/llvm-mc/llvm-mc.cpp:400
+    if (MAI->getCodePointerSize() < 8) {
+      errs() << ProgName
+             << ": the 64-bit DWARF format is only supported for 64-bit "
----------------
MaskRay wrote:
> cc/ld/binutils usually don't append a dot in a diagnostic.
> 
> They do not exist -> 32-bit targets don't support DWARF64, which requires 64-bit relocations.
> cc/ld/binutils usually don't append a dot in a diagnostic.
For reference, this is now in the LLVM style guide too (along with no upper-case letter for the start of diagnostics).


================
Comment at: llvm/tools/llvm-mc/llvm-mc.cpp:391
+  if (MCOptions.Dwarf64) {
+    // DWARFv3 was the first standard where the 64-bit DWARF was described.
+    if (DwarfVersion < 3) {
----------------
Nit: either "the 64-bit DWARF format" or simply "64-bit DWARF".


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

https://reviews.llvm.org/D80382