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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
