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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/test/MC/ELF/gen-dwarf64.s:1
+// RUN: not llvm-mc -g -dwarf-version 5 -dwarf64 -triple i686 %s -filetype=asm -o - 2>&1 | FileCheck --check-prefix=I686 %s
+// RUN: not llvm-mc -g -dwarf-version 2 -dwarf64 -triple x86_64 %s -filetype=asm -o - 2>&1 | FileCheck --check-prefix=DWARF2 %s
----------------
ELF can use `#` as well. If you want to add comments, you may use `## `


================
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 "
----------------
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.


================
Comment at: llvm/tools/llvm-mc/llvm-mc.cpp:405
+    }
+    // If needsDwarfSectionOffsetDirective is true, we would eventually call
+    // MCStreamer::emitSymbolValue() with IsSectionRelative = true, but that
----------------
dblaikie wrote:
> Perhaps this should have a FIXME? and/or the code where the 4-byte only support is should have a FIXME? Are you planning to do that in a follow-up patch?
> 
> And is this error-path tested?
Does it make sense to explicitly check for COFF?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80382