[PATCH] D82975: [DebugInfo] Allow GNU macro extension to be emitted

[David Stenberg via Phabricator via llvm-commits]

dstenb marked 2 inline comments as done.
dstenb added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:3036-3048
+      if (!Value.empty())
+        // FIXME: Add support for DWARF64.
+        Asm->OutStreamer->emitSymbolValue(
+            this->InfoHolder.getStringPool()
+                .getEntry(*Asm, (Name + " " + Value).str())
+                .getSymbol(),
+            /*Size=*/4);
----------------
dblaikie wrote:
> Might be nice to refactor this in both the original codepath and the new codepath you're adding (either before or after this commit) to compute the string once & share the rest of this expression..
> ```
> std::string Str = Value.empty() ? Name.str() : (Name + ' ' + Value).str();
> Asm->OutStreamer->emitSymbol(this->InfoHolder.getStringPool().getEntry(*Asm, Str).getSymbol(), 4);
> ```
> 
Yes, good idea! I split that out to the preparatory patch D83557.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:3088-3091
+                      dwarf::DW_MACRO_end_file, [&](unsigned Form) {
+                        return (getDwarfVersion() >= 5)
+                                   ? dwarf::MacroString(Form)
+                                   : dwarf::GnuMacroString(Form);
----------------
dblaikie wrote:
> Looks like maybe this could skip the std::function_ref, and do this:
> ```
> emitMacroFileImpl(F, U, dwarf::DW_MACRO_start_file,
>                       dwarf::DW_MACRO_end_file, 
>                         (getDwarfVersion() >= 5)
>                                    ? dwarf::MacroString
>                                    : dwarf::GnuMacroString);
> ```
Oh, thanks, of course!


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

https://reviews.llvm.org/D82975