[PATCH] D20147: [WIP] DebugInfo: New metadata representation for global variables.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: include/llvm/IR/GlobalVariable.h:32
@@ -32,1 +31,3 @@
+class DIGlobalVariable;
+class Module;
 template <typename ValueSubClass> class SymbolTableListTraits;
----------------
aprantl wrote:
> Why Module?
Just a drive-by cleanup, I was re-ordering these declarations alphabetically.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:179
@@ -188,20 +178,3 @@
     if (DD->useAllLinkageNames())
       addLinkageName(*VariableDIE, GV->getLinkageName());
   }
----------------
aprantl wrote:
> Can you remind me why this is no longer necessary?
Both of these cases are now represented using expressions: the first using DW_OP_stack_value, and the second using DW_OP_plus to apply the adjustment.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:470
@@ -475,1 +469,3 @@
+  unsigned NumDebugCUs = std::distance(M->debug_compile_units_begin(),
+                                       M->debug_compile_units_end());
   // Tell MMI whether we have debug info.
----------------
aprantl wrote:
> Nice! This should probably be a separate NFC commit.
r271360

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:491
@@ +490,3 @@
+    for (auto *GV : CUNode->getGlobalVariables()) {
+      const GlobalVariable *Global = nullptr;
+      auto I = GVMap.find(GV);
----------------
aprantl wrote:
> ```
> if (const GlobalVariable *Global = GVMap.lookup(GV))
>   CU.getOrCreateGlobalVariableDIE(GV, Global);
> ```
> ?
Yes, almost, but we still need to create DIEs for unattached globals here.

================
Comment at: lib/IR/DebugInfoMetadata.cpp:566
@@ -565,1 +565,3 @@
+    case dwarf::DW_OP_stack_value:
+      // Piece and stack value expressions must be at the end.
       return I->get() + I->getSize() == E->get();
----------------
aprantl wrote:
> Technically correct, but DW_OP_piece must be even farther at the end ;-)
> 
> In the DWARF 5 draft Figure D.61, there is an expression example that uses both to synthesize a struct from the constants {1, 2}:
> 
>         DW_OP_lit1 DW_OP_stack_value DW_OP_piece(4)
>         DW_OP_lit2 DW_OP_stack_value DW_OP_piece(4)
Okay, adjusted to say what we currently support.


http://reviews.llvm.org/D20147