[PATCH] Refactor debug info lexical block generation

Frederic Riss friss at apple.com
Tue May 26 18:30:56 PDT 2015

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.


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.



More information about the llvm-commits mailing list