[PATCH] Refactor debug info lexical block generation

[Frederic Riss]

It would be nice to have a description of how exactly the patch fixes the issue. It seems easy to understand what the patch does, but I don't see where we were losing the information in the first place (ie. how does premature coalescing lose the inner variable?)

Some other comments inline.


REPOSITORY
  rL LLVM

================
Comment at: include/llvm/CodeGen/DIE.h:191
@@ +190,3 @@
+  ///
+  void collapseChild(DIE* Child) {
+    assert(Child->getParent() == this);
----------------
This won't play nice with Duncan's rewrite of the DIE memory management that I think will land shortly.

================
Comment at: include/llvm/CodeGen/DIE.h:193-196
@@ +192,6 @@
+    assert(Child->getParent() == this);
+    for (auto &Grandchild : Child->Children) {
+      Grandchild->Parent = nullptr;
+      addChild(std::move(Grandchild));
+    }
+    Child->Parent = nullptr;
----------------
We are always adding the nodes at the end of the children list. Do we care about the order? Should we try to insert them in place of the collapsed node?

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:545-549
@@ +544,7 @@
+    bool Skip = Die->getAbbrev().getTag() == dwarf::DW_TAG_lexical_block;
+    for (auto &Child : Die->getChildren()) {
+      Dies.push_back(&*Child);
+      Skip &= (Child->getAbbrev().getTag() == dwarf::DW_TAG_lexical_block ||
+               Child->getAbbrev().getTag() == dwarf::DW_TAG_inlined_subroutine);
+    }
+
----------------
This seems prohibitively expensive, you are adding each and every DIE in the tree to the worklist. I'm pretty sure you could filter to only take subprograms/blocks/inline_subroutines. (Even better, don't we already collect all the functions somewhere?)

This also seems wrong. You might remove inlined_subroutines with that change and we don't want that.

http://reviews.llvm.org/D9960

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/