[PATCH] D43286: [CodeGen] Generate DWARF v5 Accelerator Tables

Jonas Devlieghere via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 7 08:23:00 PST 2018

JDevlieghere added a comment.

This is looking very good. A few small comment inline but I don't expect this will need a lot of additional work.

Comment at: include/llvm/MC/MCObjectFileInfo.h:258
   // DWARF5 Experimental Debug Info Sections
+  MCSection *getDwarfDebugNamesSection() const {
Obsolete now we have the "real" thing? :-) 

Comment at: lib/CodeGen/AsmPrinter/AccelTable.cpp:15
 #include "llvm/CodeGen/AccelTable.h"
+#include "DwarfCompileUnit.h"
 #include "llvm/ADT/STLExtras.h"
should be on top (clang-format should do this for you IIRC)

Comment at: lib/CodeGen/AsmPrinter/AccelTable.cpp:193
+    uint32_t AugmentationStringSize = sizeof(AugmentationString);
+    char AugmentationString[8] = {'L', 'L', 'V', 'M', '0', '7', '0', '0'};
+    static_assert(sizeof(AugmentationString) % 4 == 0, "");
Hmm, we should probably move this elsewhere and have Cmake generate it for us? This seems hard to maintain and quite a burden, wouldn't you agree?

Comment at: lib/CodeGen/AsmPrinter/AccelTable.cpp:233
+                        const DwarfDebug &DD,
+                        ArrayRef<std::unique_ptr<DwarfCompileUnit>> CompUnits);
It seems odd that we sink the DwarfCompileUnits, why's that? 

Comment at: lib/CodeGen/AsmPrinter/AccelTable.cpp:362
+  Asm->OutStreamer->EmitLabel(Ctx.ContributionStart);
+  Asm->OutStreamer->AddComment("Header: version");
+  Asm->EmitInt16(Version);
This is different from the comments we currently generate for the Apple table header where we have "Header Version". I'm fine with either (or maybe I slightly prefer this). Reminder to self that I should unify this in a follow up commit. 

Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:297
+  } else
+    AccelTableKind = DwarfAccelTables;
This doesn't seem right, if you specify apple explicitly you'll end up with DWARF? Shouldn't we just omit the else?



More information about the llvm-commits mailing list