[PATCH] D96597: [DebugInfo] Keep the DWARF64 flag in the module metadata

Tue Feb 16 11:11:51 PST 2021

dblaikie added inline comments.

================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:396
+  bool Dwarf64 =
+      (Asm->TM.Options.MCOptions.Dwarf64 || MMI->getModule()->isDwarf64()) &&
+      DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.
----------------
ikudrin wrote:
> dblaikie wrote:
> > Once this patch is accepted, it's probably worth going back and removing the MCOption? (I don't think we have MCOptions for other debug related module metadata, like the DWARF version?)
> Exactly for the DWARF version, there is a similar option, please take a look at line 388.
> As for the DWARF64 option here, it only simplifies testing with tools like `llc`, `opt`, etc. Originally, `MCTargetOptions::Dwarf64` was aimed to adjust generating in the assembler, but the corresponding command-line flag is visible in other tools, too. Do you think it is worth it to move the command line option from the common library code to specific tools?
Oh, nah, that's fine to keep around like the DWARF version for LLVM testing purposes (easier to write a test with a single .ll file and different RUN lines to test both 32 and 64 behavior, without having to have two IR Files, or rewrite the IR file to switch its format, etc)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96597