[PATCH] [prototype] Adding line table debug information to LLVM on Windows

[Eric Christopher]

  In general it's starting to look very nice, thanks. Few comments in there plus the other comments from Rafael and David.

  FWIW I do like the test cases for the larger debug info using the dumping facility to make it easier to figure out what's going on.


================
Comment at: include/llvm/MC/MCObjectFileInfo.h:132
@@ -131,1 +131,3 @@
 
+  const MCSection *COFFDebugInfo;
+
----------------
Is all COFF debug info in one section or just the line tables?

In general adding the sections to the MC layer can just be added as a separate small patch.

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.h:37
@@ +36,3 @@
+
+  struct FunctionDI {
+    SmallVector<MCSymbol *, 10> Instrs;
----------------
Comment of what this is and what it's used for.

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.h:48
@@ +47,3 @@
+
+  struct InstrInfoTy {
+    StringRef Filename;
----------------
Comment of what this is and what it's used for.

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.h:59
@@ +58,3 @@
+
+  struct FileNameRegistryTy {
+    SmallVector<StringRef, 10> Filenames;
----------------
Comment of what this is and what it's used for.

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.h:71
@@ +70,3 @@
+
+    void add(StringRef Filename) {
+      if (Infos.count(Filename))
----------------
Comments.

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp:127
@@ +126,3 @@
+
+  Asm->EmitInt32(4);  // Starting magic number.
+
----------------
Magic!

How about an enum defined somewhere?

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp:245
@@ +244,3 @@
+
+// TODO: This function was copied from DwardDebug.cpp. Any reason not to move
+// the code to DebugLoc.h?
----------------
Typo.

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp:246
@@ +245,3 @@
+// TODO: This function was copied from DwardDebug.cpp. Any reason not to move
+// the code to DebugLoc.h?
+// Walk up the scope chain of given debug loc and find line number info
----------------
No particular reason I don't think.

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp:136
@@ +135,3 @@
+  // which holds the PC to file:line table.
+  for (size_t I = 0, E = VisitedFunctions.size(); I != E; ++I) {
+    const Function *GV = VisitedFunctions[I];
----------------
If you could split this loop into functions it would make it a bit easier.


http://llvm-reviews.chandlerc.com/D2232