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

[David Stenberg via Phabricator via llvm-commits]

dstenb marked an inline comment as done.
dstenb added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1359-1368
+        dwarf::Attribute MacrosAttr = getDwarfVersion() >= 5
+                                          ? dwarf::DW_AT_macros
+                                          : dwarf::DW_AT_GNU_macros;
         if (useSplitDwarf())
           TheCU.addSectionDelta(
-              TheCU.getUnitDie(), dwarf::DW_AT_macros, U.getMacroLabelBegin(),
+              TheCU.getUnitDie(), MacrosAttr, U.getMacroLabelBegin(),
               TLOF.getDwarfMacroDWOSection()->getBeginSymbol());
----------------
dblaikie wrote:
> dstenb wrote:
> > dblaikie wrote:
> > > Looks like this might be wrong for v4 + split DWARF + using macro? Or perhaps this code isn't reachable by that combination?
> > > 
> > > Might be more clear, then, to sink the MacrosAttr choice down into the "else" clause here, and assert in the split DWARF case that the version >= 5? (possibly including a note about how the pre-v5, GCC debug_macro extension isn't supported with Split DWARF)
> > Sorry, in what way does this look wrong? If I am not overlooking something, this look the same as what GCC emits for the attribute in the `-g3 -gdwarf-4 -gsplit-dwarf` case.
> > 
> > Regardless of the above, doing like you suggest and adding an assert seems like a good idea. 
> > Sorry, in what way does this look wrong? If I am not overlooking something, this look the same as what GCC emits for the attribute in the -g3 -gdwarf-4 -gsplit-dwarf case.
> 
> I think that's what we were discussing at length previously in this review, that GNU debug_macro + v4 + split DWARF seems a bit ill specified, and it's probably best to have -fdebug-macro + v4 + -gsplit-dwarf continue to use debug_macinfo rather than the ill-specified and not-implemented-by-any-consumer v4 Split DWARF .debug_macro?
> 
> https://reviews.llvm.org/D82975#2142264
> 
Okay, yes, I guess it's bad to assume that the attribute part is valid if the whole GNU debug_macro + v4 + split DWARF format seems ill specified. I have added an assert as you suggested.


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

https://reviews.llvm.org/D82975