[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.
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/
More information about the llvm-commits
mailing list