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

[Timur Iskhodzhanov]

  Please take another look!


================
Comment at: include/llvm/MC/MCObjectFileInfo.h:132
@@ -131,1 +131,3 @@
 
+  const MCSection *COFFDebugInfo;
+
----------------
Eric Christopher wrote:
> 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.
Good catch - it should actually be `COFFDebugSymbolsSection`.
Not sure adding it in a separate patch is worth it.

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp:127
@@ +126,3 @@
+
+  Asm->EmitInt32(4);  // Starting magic number.
+
----------------
Eric Christopher wrote:
> Magic!
> 
> How about an enum defined somewhere?
Yikes, fixed!

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.h:37
@@ +36,3 @@
+
+  struct FunctionDI {
+    SmallVector<MCSymbol *, 10> Instrs;
----------------
Eric Christopher wrote:
> Comment of what this is and what it's used for.
Renamed to `FunctionInfo` and added a comment.

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

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

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

================
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?
----------------
Eric Christopher wrote:
> Typo.
Addressed the TODO, so no more typo needed :o)

================
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
----------------
Eric Christopher wrote:
> No particular reason I don't think.
ditto

================
Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp:256
@@ +255,3 @@
+    // TODO: What's the 19 magic number? Is it a DWARF limitation or the way to
+    // distinguish between what and what?
+    if (SP->getNumOperands() > 19)
----------------
Replaced with a FIXME to name the magic constant.

================
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];
----------------
Eric Christopher wrote:
> If you could split this loop into functions it would make it a bit easier.
Done, also improved the comments a little bit.


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