[PATCH] D16292: Improved Macro emission in dwarf

[David Blaikie via llvm-commits]

dblaikie added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:565
@@ +564,3 @@
+  MCSection *Macinfo = TLOF.getDwarfMacinfoSection();
+  if (Macinfo && !Macinfo->getBeginSymbol()) {
+    MCSymbol *MacroSectionStartSym = Asm->createTempSymbol("macro");
----------------
We don't support non-existence of other DWARF sections (we don't generally check for null return from TLOF.getDwarf* functions elsewhere, so far as I can see/recall) - is there any reason we need to do it in this particular case?

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:566
@@ +565,3 @@
+  if (Macinfo && !Macinfo->getBeginSymbol()) {
+    MCSymbol *MacroSectionStartSym = Asm->createTempSymbol("macro");
+    Macinfo->setBeginSymbol(MacroSectionStartSym);
----------------
I think this is the wrong API (looking at how setBeginSymbol is called elsewhere) to create the section start symbol - but in any case, again, we don't create section start symbols anywhere in the DWARF codegen, so I don't think we need to do it for Macro info either. We just rely on their existence in all other cases, it seems?

It's probably worth doing a little test of your own to build two separate objects with macro info, link them together & check that dumping/values all make sense - that the two CU's references to their macinfo actually get relocated during linking, etc. (no need to commit a test like this - we could have a test that checks that there is a relocation for the macinfo attribute, but I'm not /too/ fussed about that, it's awkward and probably annoyingly platform specific, etc)

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:629
@@ -626,3 +628,3 @@
     }
   }
 
----------------
addSectionDelta isn't quite right - because it doesn't account for the MachO case where relocations are not used.

DwarfCompileUnit::addSectionLabel handles this case and wraps addSectionDelta.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1511
@@ -1508,3 +1510,3 @@
 
-// Emit visible names into a debug str section.
+/// Emit null-terminated strings into a debug str section.
 void DwarfDebug::emitDebugStr() {
----------------
Please commit these unrelated comment fixes separately. No need for further review on that, you can just commit them directly yourself.


http://reviews.llvm.org/D16292